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

[MAINT] Trying to re-enable testing for non-x86 archs #3149

Merged
merged 17 commits into from
Mar 13, 2021

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Mar 12, 2021

Partially fixes MDAnalysis/mdaencore#38 and #3127

Changes made in this Pull Request:

  • Make encore compile args a separate list so we can selectively reduce optimisation (it's the only part of the code that seems more sensitive to floating point errors).
  • Adds Travis cron jobs for ppc64le and aarch64

PR Checklist

  • Tests?
  • Docs? - N/A?
  • CHANGELOG updated?
  • Issue raised/referenced?

@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #3149 (266bf61) into develop (a35608f) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    MDAnalysis/mdanalysis#3149      +/-   ##
===========================================
- Coverage    92.68%   92.68%   -0.01%     
===========================================
  Files          171      168       -3     
  Lines        22737    22677      -60     
  Branches      3212     3212              
===========================================
- Hits         21074    21018      -56     
+ Misses        1615     1611       -4     
  Partials        48       48              
Impacted Files Coverage Δ
package/MDAnalysis/coordinates/__init__.py 100.00% <0.00%> (ø)
package/MDAnalysis/coordinates/chemfiles.py 31.81% <0.00%> (ø)
package/MDAnalysis/analysis/encore/cutils.pyx
...e/dimensionality_reduction/stochasticproxembed.pyx
...alysis/analysis/encore/clustering/affinityprop.pyx

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 a35608f...266bf61. Read the comment docs.

@IAlibay
Copy link
Member Author

IAlibay commented Mar 12, 2021

closing & re-opening due to some GH issues that have stalled updating my latest commits.

@IAlibay IAlibay closed this Mar 12, 2021
@IAlibay IAlibay reopened this Mar 12, 2021
@IAlibay
Copy link
Member Author

IAlibay commented Mar 12, 2021

That's weird, looks like conda is picking up an old version of scipy even thought numpy is picked up as 1.20. I can pin CI to >1.5 I guess. Any thoughts @tylerjereddy ?

https://travis-ci.com/github/MDAnalysis/mdanalysis/jobs/490647828#L2460

@tylerjereddy
Copy link
Member

Hmm, one difference is that SciPy is listed explicitly while NumPy is not (presumably implicit), for the conda install line?

I wouldn't think that would be the cause though; the ppc64le arch is certainly not as well supported, but there seems to be an appropriate conda binary for modern SciPy on that platform here: https://repo.anaconda.com/pkgs/main/linux-ppc64le/

@IAlibay
Copy link
Member Author

IAlibay commented Mar 12, 2021

Hmm, one difference is that SciPy is listed explicitly while NumPy is not (presumably implicit), for the conda install line?

I wouldn't think that would be the cause though; the ppc64le arch is certainly not as well supported, but there seems to be an appropriate conda binary for modern SciPy on that platform here: https://repo.anaconda.com/pkgs/main/linux-ppc64le/

Strange it does pick it up if told to (or afterwards if doing it interactively). I won't question it too much, we can always pin the versions when we release on conda-forge.

@IAlibay
Copy link
Member Author

IAlibay commented Mar 12, 2021

The provenance of 98 new user warnings is baffling to me: https://github.com/MDAnalysis/mdanalysis/pull/3149/checks?check_run_id=2099327091#step:10:317

@IAlibay IAlibay marked this pull request as ready for review March 13, 2021 03:28
name: "Power PC"
os: linux
arch: ppc64le
if: type = cron
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll be honest, I'm not 100% sure this will work. I have enabled the cron job on develop to be daily for now (eventually we can change it to weekly). So maybe it'll get triggered tomorrow, but it's not clear to me that the cron job will run if this doesn't get merged first.

- python: 3.8
name: "Power PC"
os: linux
arch: ppc64le
Copy link
Member Author

Choose a reason for hiding this comment

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

We can probably add s390x here for the sake of #1389, but is IBM Z really that prevalent? I guess the z15 was released in 2019, but I can't think of a single cluster that uses it? (definitely nothing in the top500 list)

@IAlibay
Copy link
Member Author

IAlibay commented Mar 13, 2021

Cheers @richardjgowers I'll merge so we can test that today's cron job is working as intended. We can discuss s390x further in MDAnalysis/mdaencore#38

@IAlibay IAlibay merged commit 3f589e3 into MDAnalysis:develop Mar 13, 2021
@IAlibay IAlibay deleted the non-x86 branch March 13, 2021 19:46
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this pull request Mar 30, 2021
…lysis#3149)

Partially fixes #1389 and MDAnalysis#3127

# Work done in this PR
- Adds basic support for ppc64le
- Adds encore specific C compiler args to reduce optimisations in non-x86 systems
- Enables travis cron CI for aarch64 and ppc64le
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants