EddyRivasLab / hmmer

HMMER: biological sequence analysis using profile HMMs
http://hmmer.org
Other
307 stars 69 forks source link

Add platform-specific implementation for ARM using NEON #233

Closed althonos closed 3 years ago

althonos commented 3 years ago

Supersedes #230, rebased to h3-arm.

You'll need easel checked out on the master branch to build. Compiles and passes all tests on my Raspberry Pi 4.

npcarter commented 3 years ago

Pull request looks very good, only saw a few minor issues that I'll address. Nice job!

althonos commented 3 years ago

Thanks! Sorry about the few whitespaces trimmed here and there, I had my editor setup with line trimming... I rolled back the files that had the most meaningless changes, but there are definitely some left in the diff!

npcarter commented 3 years ago

Not a problem on the white spaces. Also, I wanted to let you know that it looks like your SSV port was very close, and just used a non-saturating subtract where it should have used a saturating subtract. With that change, the NEON version of H3 passes the base test suite, and I'm moving on to more serious tests and checking some of the uncommon compilation modes like MPI and debugging.

-Nick

On Tue, Mar 30, 2021 at 6:52 PM Martin Larralde @.***> wrote:

Thanks! Sorry about the few whitespaces trimmed here and there, I had my editor setup with line trimming... I rolled back the files that had the most meaningless changes, but there are definitely some left in the diff!

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/EddyRivasLab/hmmer/pull/233#issuecomment-810630215, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABDJBZFNZDEUDYKJACQ4BSTTGJI3TANCNFSM4Z4LTR7Q .

althonos commented 3 years ago

Amazing, thanks!

boegel commented 3 years ago

Is there an ETA for an HMMER3 release that includes these changes?

martin-g commented 2 years ago

Would it be a good idea to enable TravisCI to run the build and tests on ARM64 ? I would be happy to send a PR for this! I see there is https://github.com/EddyRivasLab/hmmer/blob/master/.travis.yml but it seems it is not enabled at TravisCI (https://app.travis-ci.com/github/EddyRivasLab/hmmer) even for x86_64 at the moment.

cryptogenomicon commented 2 years ago

Thanks, but since we have a system for testing already, we haven't found TravisCI to be worth the extra effort.

martin-g commented 2 years ago

Thank you for your response, @cryptogenomicon !

since we have a system for testing already

I knew it there must be something else! Does this system also have a builder on ARM64 ?

cryptogenomicon commented 2 years ago

Sorry, I don't know what you mean by "this system" or "a builder".

Our make check should run on any platform we support, including ARM64 platforms for the ARM dev branch.

martin-g commented 2 years ago

Because we were talking about TravisCI I thought that by "we have a system for testing already" you mean a CI running on internal machine(s), e.g. Jenkins/Buildbot/.... By "builder" I meant a CI agent running on Linux ARM64.

cryptogenomicon commented 2 years ago

We don't use CI. It seems like too much additional overhead for the scale of our project.