Skip to content

Add aarch64, ppc64le and s390x cases to Travis CI#102

Closed
junaruga wants to merge 1 commit intoBenLangmead:masterfrom
junaruga:feature/support-multi-cpus
Closed

Add aarch64, ppc64le and s390x cases to Travis CI#102
junaruga wants to merge 1 commit intoBenLangmead:masterfrom
junaruga:feature/support-multi-cpus

Conversation

@junaruga
Copy link

@junaruga junaruga commented Jan 6, 2020

I have a suggestion.

This PR is to add aarch64, ppc64le and s390x cases to Travis CI, updating Makefile to build each arch easily. It is related to #13 and #95 .
How do you think?

There are 2 commits in the PR.
First commit is to fiix following error on s390x.

alphabet.cpp:293:1: error: narrowing conversion of ‘-1’ from ‘int’ to ‘char’ inside { } [-Wnarrowing]

This prevents to build on s390x. You can see this Travis log on my forked repository for detail.

Here is my forked repository's Travis CI result.

One note is right now for s390x, only make allall is executed because make simple-test fails.
You can see this Travis log for detail.

./bowtie-build  --threads 2 --quiet --sanity  .simple_tests.pl.fa .simple_tests.tmp
Bad exitlevel from bowtie-build: 139 at ./scripts/test/simple_tests.pl line 1141.

Dear maintainers, do you have any idea to fix this?

@mr-c do you know how you built bowtie for Debian bowtie s390x package? Have you run the make simple-test on s390x?
Do we need to apply one of the Debian patche files to this repository?

Best regards,
Jun

@junaruga
Copy link
Author

junaruga commented Jan 7, 2020

This repository's Travis builds are running here.
But the link is not shown in this page. I do not know why.
https://travis-ci.org/BenLangmead/bowtie/builds/633533222

@junaruga
Copy link
Author

junaruga commented Jan 7, 2020

I found another pull-request fixing the issue in alphabet.cpp and adding aarch64 support here.
#95

@mr-c
Copy link

mr-c commented Jan 7, 2020

@junaruga In Debian, we fail at building bowtie2 on s390x, armel, armhf, i386, and mipsel along with some unsupported architectures: https://buildd.debian.org/status/package.php?p=bowtie2

I will try your patch for s390x!

@junaruga
Copy link
Author

junaruga commented Jan 7, 2020

@mr-c Thanks for sharing the building status about bowtie2.
This PR is for bowtie, not for bowtie2. I think the alphabet.cpp's patch fixes issue for bowtie2 too.
So, seeing the status about bowtie. It seems the you succeeded at building bowtie at s390x, right?
Have you been running make simple-test in the building process?
https://buildd.debian.org/status/package.php?p=bowtie

@mr-c
Copy link

mr-c commented Jan 7, 2020

@junaruga Sorry for the bowtie/bowtie2 confusion. I didn't work on the Debian bowtie package, so I can't say much. Looking at the patches the biggest thing is that the embedded copy of seqan was upgraded to seqan 1.4.2 and seqan's portable popcount method was used instead of bowtie's.

Have you been running make simple-test in the building process?

No, but I will try that

@mr-c
Copy link

mr-c commented Jan 8, 2020

@junaruga
so s390x builds, and most of the tests pass, but bowtie 1.2.3+dfsg-3 segfaults with 2+ threads on s390x:
https://buildd.debian.org/status/fetch.php?pkg=bowtie&arch=s390x&ver=1.2.3%2Bdfsg-3&stamp=1578504914&file=log

@mr-c
Copy link

mr-c commented Jan 8, 2020

(a qemu s390x build passed the tests when threads are limited to just one)

@junaruga
Copy link
Author

junaruga commented Jan 9, 2020

@mr-c Thank you for checking it on Debian build system.
It's interesting that the behavior of the Debian's s390x tests is different from the Travis s390x.
And the threads and QEMU information are useful too.

@junaruga
Copy link
Author

junaruga commented Jan 9, 2020

I didn't work on the Debian bowtie package, so I can't say much. Looking at the patches the biggest thing is that the embedded copy of seqan was upgraded to seqan 1.4.2 and seqan's portable popcount method was used instead of bowtie's.

And also thank you for sharing the situation in Debian.

@mr-c
Copy link

mr-c commented Jan 10, 2020

@junaruga You are welcome. I'm sure the different result is due to the extensive patching we did to update to newer Seqan. I'm happy to contribute the patches to this repo, if there is interest and willingness to merge them.

* Update aligning with bowtie2 .travis.yml and Makefile.
* Add osx case, gccN cases.
@junaruga junaruga force-pushed the feature/support-multi-cpus branch from f48af10 to 30d4939 Compare February 28, 2020 14:10
@junaruga
Copy link
Author

Hi @ch4rr0 Congrats, new release for bowtie2. I rebased this PR aligning it with bowtie2 master's .traivs.yml and Makefile , adding gcc-N case, osx case and other CPU architecture cases.

Here is the Travis CI result.

Some notes

  • In case of bowtie, Python scripts exists on ./scripts/test/*.py (#!/usr/bin/env python). But in my understanding, as those are not executed from Travis make simple-test, I did not use language: python to run it on Python 3 for the test cases.
  • For s390x ppc64le, there are test failures for make simple-test. In this PR, only running make allall on the CPU architectures to pass Travis. But you can run s390x and ppc64le make simple-test` allowing the failures like this.
$ git diff
diff --git a/.travis.yml b/.travis.yml
index 6abc003..8732fe4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -52,8 +52,6 @@ matrix:
       env:
         - POPCNT_CAPABILITY=0
         - NO_TBB=1
-      # Skip make simple-test as it fails.
-      script: make -j $NPROC allall
     # PPC64LE
     - os: linux
       arch: ppc64le
@@ -61,8 +59,10 @@ matrix:
       env:
         - POPCNT_CAPABILITY=0
         - NO_TBB=1
-      # Skip make simple-test as it fails.
-      script: make -j $NPROC allall
+  allow_failures:
+    - arch: s390x
+    - arch: ppc64le
+  fast_finish: true
 cache: apt
 env:
   global:

MACS's .travis.yml and Travis result might be a good example for you to see the allow_failures syntax of .travis.yml.

I wish you like this PR.

@junaruga
Copy link
Author

But in my understanding, as those are not executed from Travis make simple-test, I did not use language: python to run it on Python 3 for the test cases.

Sorry my mistake. Python is used in bowtie, bowtie-build and bowtie-inspect as bowtie2 as well. But modifying the code from python to python3 could be another PR. I would prefer this PR is for only .travis.yml and Makefile.

$ grep -rl python *
bowtie
bowtie-build
bowtie-inspect
scripts/test/large_idx.py
scripts/test/build_big.py
scripts/test/btdata.py
scripts/test/dataface.py
scripts/test/btface.py

This was referenced Feb 28, 2020
@ch4rr0
Copy link
Collaborator

ch4rr0 commented Jul 17, 2020

I copied over the .travis.yml you contributed to the bowtie2 repo. With a few minor changes we see that all builds and simple-tests succeed with the exception of s390x. I’ll investigate why that is.

@ch4rr0
Copy link
Collaborator

ch4rr0 commented Jul 18, 2020

I fixed the s390x bug. The simple tests now pass for all architectures.

@junaruga
Copy link
Author

Thanks! I close this PR, as Travis has the 3 architecture cases by d20447f .

I fixed the s390x bug. The simple tests now pass for all architectures.

@ch4rr0 I assume the fix is this commit. 4fbac0e

How did you fix it? Because if you did not have the native local s390x machine, it was hard to debug and fix it.

@junaruga junaruga closed this Jul 25, 2020
@ch4rr0
Copy link
Collaborator

ch4rr0 commented Jul 25, 2020

I used the s390x instance in Travis to help debug and fix the issue. Since I don’t have direct access to the node I used print statements to verify inputs/outputs.

@junaruga junaruga deleted the feature/support-multi-cpus branch July 25, 2020 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants