Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed gromacs package for testing #130

Closed
wants to merge 1 commit into from
Closed

Conversation

ianmkenney
Copy link
Member

Moved from becksteinlab to bioconda package

@orbeckst orbeckst self-assigned this May 9, 2018
@orbeckst orbeckst added this to the 0.7 milestone May 9, 2018
@codecov-io
Copy link

codecov-io commented May 9, 2018

Codecov Report

Merging #130 into develop will increase coverage by 0.12%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #130      +/-   ##
===========================================
+ Coverage    48.14%   48.27%   +0.12%     
===========================================
  Files           24       24              
  Lines         4831     4831              
  Branches       731      731              
===========================================
+ Hits          2326     2332       +6     
+ Misses        2352     2340      -12     
- Partials       153      159       +6
Impacted Files Coverage Δ
gromacs/tools.py 54.01% <0%> (-0.73%) ⬇️
gromacs/utilities.py 53.05% <0%> (-0.33%) ⬇️
gromacs/config.py 65.8% <0%> (+0.64%) ⬆️
gromacs/core.py 57.75% <0%> (+3.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebe740e...2f9a0f3. Read the comment docs.

Moved from becksteinlab to bioconda package
@orbeckst orbeckst mentioned this pull request May 10, 2018
# install bioconda Gromacs build... Need to set up channels first
# per instructions at: https://bioconda.github.io/

- conda config --add channels defaults
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaults is always on by default.

@@ -1,8 +1,8 @@
name: gw
channels:
- becksteinlab
- bioconda
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you also need to add conda-forge for the version 2018 dependencies

@kain88-de
Copy link
Contributor

maybe continuing to test gromacs 5.x from the beckstein lab repo would be good. there is still a large number of people using gromacs 5

@orbeckst
Copy link
Member

The problem with the 5.x build is that it is currently tied to Python 2; we would need to build a Python 3 version (or get rid of any Python dependencies - which is the get_gmx script). We hope to not have to maintain the gromacs conda packages any more...

@orbeckst
Copy link
Member

@ianmkenney said that the Bioconda gromacs 2018 core dumps on Travis – perhaps an issue with the instruction set not matching what is available on Travis??

@ianmkenney can you post the evidence that you have?

@kain88-de
Copy link
Contributor

I can imagine where the core dumps come from. The travis/circleci build on conda forge was done with a random CPU and GROMACS always chooses the best available SIMD instruction set. On travis we can be randomly assigned an older CPU that doesn't support the instruction set. The only way around this is to specify a common instruction set like SSE3 during the cmake step. Making our own package on conda-forge can help to sort out these issues.

About maintenance. Using bioconda and conda-forge doesn't get rid of it completely but sharing responsibility is much easier.

@ianmkenney
Copy link
Member Author

log with the error

I put gmx grompp --help and got an illegal instruction error

$ gmx grompp --help
/home/travis/.travis/job_stages: line 57:  2239 Illegal instruction     (core dumped) gmx grompp --help

@kain88-de
Copy link
Contributor

yeah that is a instruction issue. But that is fixable upstream

@orbeckst
Copy link
Member

orbeckst commented May 11, 2018

WHat is the best approach to proceed? Raise an issue with bioconda and ask them to build Gromacs taking portability into account? E.g. build binaries with instruction set suffixes

  • gmx_sse2
  • gmx_avx
  • etc

or just build gmx with sse2?

@kain88-de
Copy link
Contributor

I'm submitted a small PR.

@kyleabeauchamp
Copy link

There is an ongoing project to upgrade Bioconda to Conda-build3 and also to figure out a way to build multiple variants of a package, with different optimization levels. Please take a look as Gromacs is a prime candidate for specifying multiple builds. AFAIK, I think the conda-forge folks are going to start assuming that SSE3 is available as the default.

bioconda/bioconda-recipes#2354

conda-forge/conda-forge.github.io#49

@kain88-de
Copy link
Contributor

I would have gone for SSE3 but gromacs doesn't over it. I'm aware of the conda-build3 transition happening in conda-forge. It will still take a little while for them to make it though.

@kyleabeauchamp
Copy link

SSE2 SGTM then

@kain88-de
Copy link
Contributor

kain88-de commented May 11, 2018

@ianmkenney you should be able to try in about an hour.

@rhheilma
Copy link
Collaborator

rhheilma commented May 11, 2018 via email

@kain88-de
Copy link
Contributor

@rhheilma you have to change your GitHub subscription setting yourself. We cannot do this for you.

@orbeckst
Copy link
Member

The Python 3 PR #127 includes the new bioconda packages so we can fix everything there (and close this PR).

Thanks @kain88-de for fixing upstream.

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.

6 participants