[MAINT] Remove conda from CI Linux Workflows #794

kmilo9999 · 2024-06-12T23:25:17Z

The Linux build in the CI is failing because conda is installing a version of neuron without MPI support. Removing conda solves this issue for the linux CI workflow.

Solves #780

jasmainak · 2024-06-13T01:07:58Z

.github/workflows/unit_tests.yml

+              brew install open-mpi
+              echo "MPI_LIB_NRN_PATH=/opt/homebrew/lib/libmpi.dylib" >> $GITHUB_ENV


this is not what we recommend for the users though ...

I'm open to any suggestions

Conda does not work? It seems to work on my mac and 2 other macs we tried on today ...

The Linux build in the CI is failing because conda is installing a version of neuron without MPI support. Removing conda solves this issue in Linux, but it affects the mac-os build. This PR helps giving support to both systems without conda.

have you read the parallel backend guide:

https://jonescompneurolab.github.io/hnn-core/stable/parallel.html

? You should use conda for mac but pip for linux

I split the os build and test on different workflows

jasmainak · 2024-06-13T16:40:03Z

.github/workflows/unit_tests.yml

-          activate-environment: test
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v5
+        id: cp310


what is cp310?

It's an id to refer the action in further steps. We dont need this anymore, I removed it.

codecov-commenter · 2024-06-14T03:27:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.49%. Comparing base (819f4f4) to head (9a94859).
Report is 63 commits behind head on master.

❗ Current head 9a94859 differs from pull request most recent head 1b454ea

Please upload reports for the commit 1b454ea to get more accurate results.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #794      +/-   ##
==========================================
+ Coverage   92.33%   92.49%   +0.16%     
==========================================
  Files          27       27              
  Lines        5059     5343     +284     
==========================================
+ Hits         4671     4942     +271     
- Misses        388      401      +13

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gtdang · 2024-06-14T19:02:01Z

.github/workflows/linux_unit_test.yml

+    strategy:
+      matrix:
+        os: [ubuntu-latest]
+        python-version: [3.8,3.9]


Support for python 3.8 will end by the end of this year. Should we consider also testing more recent' versions of python in the matrix? At some point do we plan to stop testing for older versions? How do we currently communicate with users which python versions are actually supported by hnn?

@jasmainak @rythorpe @ntolley

https://devguide.python.org/versions/

Perhaps let's add up to 3.11 until we figure out what's going on with 3.12?

Also in terms of communication, perhaps we can explicitly state in the install instructions which versions of python we have listed in the unit tests here. Something like "HNN-core has been tested on python versions 3.8 - 3.11. Other versions may work but we do not explicitly test against them".

@hollandjg Do you recall how autora does this? I thought there was a badge with tested python versions at one point, I don't see it anymore. Do they have a methodology for supporting versions?

ntolley

This looks great to me! Perhaps we can try adding 3.10 and 3.11 to the list and see if the unit tests still pass?

ntolley · 2024-06-16T16:22:33Z

Looks like 3.11 is failing with the same error as 3.12 mentioned in #796 🫤

kmilo9999 · 2024-06-16T18:41:14Z

Looks like 3.11 is failing with the same error as 3.12 mentioned in #796 🫤

I believe the test hnn_core/tests/test_parallel_backends.py:test_mpi_failure fails on Python versions 3.9, 3.10, and 3.11 across all runners

kmilo9999 · 2024-06-17T15:10:11Z

@gtdang @ntolley @jasmainak
The reason build-python 3.9-3.11 fails it's because It appears that NumPy 2.0.0 was released last week, and they have removed the deprecated function np.in1d. This function is used in multiple parts of our project

What should be the next steps?
It doesnt fail in python 3.8 because it uses Numpy 1.94
Numpy 2.0.0 supports python versions 3.9-3.12.

kmilo9999 · 2024-06-20T19:53:15Z

@gtdang @ntolley @jasmainak It seems 3.11 and 3.12 fail at the same point, but I can take care of it on another PR.
This one works on 3.8, 3.9 and 3.10. Please review it.

ntolley

This is great @kmilo9999!! Thanks a bunch for taking care of this. Definitely agree to focus on 3.11 and 3.12 in a separate PR.

Happy to merge if it's all set on your end

jasmainak · 2024-06-20T20:00:22Z

Btw, in mne-python etc., we make our CIs fail if there are deprecation warnings ... it catches these issues early.

jasmainak · 2024-06-20T20:03:31Z

Looks good. What is your thought on two separate runner scripts vs a single one? A common code path (with if-else in appropriate places) would reduce likelihood of bugs ...

kmilo9999 · 2024-06-21T15:59:07Z

@gtdang @ntolley @jasmainak I removed the linux_unit_tests.yml , and I put linux and mac workflows in the same file with conditionals around the required steps for each OS.
Please review.

jasmainak · 2024-06-21T16:03:59Z

Beautiful, thanks @kmilo9999 !!

* removed conda from ci pipeline * MAINT: Switching to ubuntu 22.04 * MAINT: Added conditionals to install openmpi according to matrix.os * MAINT: moving macos MPI_LIB_NRN_PATH env variable assigment * MAINT: Removing comments and additional test in unit_test.yml * MAINT: Moving linux CI to a separate workflow * STY: fixed flake8 errors * STY: fixed flake8 errors in gui.py * MAINT: Removing linux commands from macos CI workflow * TST: Adding python 3.10 and 3.11 to the tests * TST: Adding quotes to python version 3.10 * TST: Adding double quotes for macos python 3.10 build * MAINT: removing python 3.11 from CI tests * MAINT: Testing Ifs in workflow to test multiple OS * MAINT: Fixed checkout@v4 statement * MAINT: Adding python 3.9 and 3.10 to multi porpuse workflow * MAINT: Removed linux workflow file. Moved logic to unix_unit_tests.yml

jasmainak reviewed Jun 13, 2024

View reviewed changes

kmilo9999 force-pushed the fix-ubuntu-ci branch from 12379bb to 862fb41 Compare June 14, 2024 03:05

kmilo9999 changed the title ~~[MAINT] Remove conda from Github CI Workflows~~ [MAINT] Remove conda from CI Linux Workflows Jun 14, 2024

kmilo9999 requested review from gtdang and ntolley June 14, 2024 03:37

kmilo9999 marked this pull request as ready for review June 14, 2024 03:38

kmilo9999 requested a review from jasmainak June 14, 2024 03:40

kmilo9999 self-assigned this Jun 14, 2024

gtdang reviewed Jun 14, 2024

View reviewed changes

ntolley approved these changes Jun 16, 2024

View reviewed changes

ntolley mentioned this pull request Jun 17, 2024

[MRG] Replace np.in1d() with np.isin() #799

Merged

kmilo9999 added 12 commits June 20, 2024 14:52

removed conda from ci pipeline

00eec5e

MAINT: Switching to ubuntu 22.04

0b90489

MAINT: Added conditionals to install openmpi according to matrix.os

9792723

MAINT: moving macos MPI_LIB_NRN_PATH env variable assigment

848c69b

MAINT: Removing comments and additional test in unit_test.yml

a487ed6

MAINT: Moving linux CI to a separate workflow

dee1f3a

STY: fixed flake8 errors

3b07028

STY: fixed flake8 errors in gui.py

e7b7898

MAINT: Removing linux commands from macos CI workflow

9c4f8bc

TST: Adding python 3.10 and 3.11 to the tests

5191eca

TST: Adding quotes to python version 3.10

2a8dc3d

TST: Adding double quotes for macos python 3.10 build

852269a

kmilo9999 force-pushed the fix-ubuntu-ci branch from d4a1150 to 852269a Compare June 20, 2024 19:04

MAINT: removing python 3.11 from CI tests

fe7e3c6

kmilo9999 requested review from gtdang and ntolley June 20, 2024 19:53

ntolley approved these changes Jun 20, 2024

View reviewed changes

kmilo9999 linked an issue Jun 20, 2024 that may be closed by this pull request

CI: GH Linux runner stalling on pytest #780

Closed

kmilo9999 added 4 commits June 21, 2024 10:49

MAINT: Testing Ifs in workflow to test multiple OS

afb34ee

MAINT: Fixed checkout@v4 statement

db2794d

MAINT: Adding python 3.9 and 3.10 to multi porpuse workflow

c78cb07

MAINT: Removed linux workflow file. Moved logic to unix_unit_tests.yml

1b454ea

kmilo9999 requested a review from ntolley June 21, 2024 15:59

jasmainak merged commit 37e9e08 into jonescompneurolab:master Jun 21, 2024
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MAINT] Remove conda from CI Linux Workflows #794

[MAINT] Remove conda from CI Linux Workflows #794

kmilo9999 commented Jun 12, 2024 •

edited

Loading

jasmainak Jun 13, 2024

kmilo9999 Jun 13, 2024

jasmainak Jun 13, 2024

kmilo9999 Jun 13, 2024 •

edited

Loading

jasmainak Jun 13, 2024

kmilo9999 Jun 14, 2024

jasmainak Jun 13, 2024

kmilo9999 Jun 14, 2024 •

edited

Loading

codecov-commenter commented Jun 14, 2024 •

edited

Loading

gtdang Jun 14, 2024

ntolley Jun 16, 2024

gtdang Jun 17, 2024

ntolley left a comment

ntolley commented Jun 16, 2024

kmilo9999 commented Jun 16, 2024 •

edited

Loading

kmilo9999 commented Jun 17, 2024 •

edited

Loading

kmilo9999 commented Jun 20, 2024 •

edited

Loading

ntolley left a comment •

edited

Loading

jasmainak commented Jun 20, 2024

jasmainak commented Jun 20, 2024

kmilo9999 commented Jun 21, 2024

jasmainak commented Jun 21, 2024

		brew install open-mpi
		echo "MPI_LIB_NRN_PATH=/opt/homebrew/lib/libmpi.dylib" >> $GITHUB_ENV

[MAINT] Remove conda from CI Linux Workflows #794

[MAINT] Remove conda from CI Linux Workflows #794

Conversation

kmilo9999 commented Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmilo9999 Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmilo9999 Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Jun 14, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntolley left a comment

Choose a reason for hiding this comment

ntolley commented Jun 16, 2024

kmilo9999 commented Jun 16, 2024 • edited Loading

kmilo9999 commented Jun 17, 2024 • edited Loading

kmilo9999 commented Jun 20, 2024 • edited Loading

ntolley left a comment • edited Loading

Choose a reason for hiding this comment

jasmainak commented Jun 20, 2024

jasmainak commented Jun 20, 2024

kmilo9999 commented Jun 21, 2024

jasmainak commented Jun 21, 2024

kmilo9999 commented Jun 12, 2024 •

edited

Loading

kmilo9999 Jun 13, 2024 •

edited

Loading

kmilo9999 Jun 14, 2024 •

edited

Loading

codecov-commenter commented Jun 14, 2024 •

edited

Loading

kmilo9999 commented Jun 16, 2024 •

edited

Loading

kmilo9999 commented Jun 17, 2024 •

edited

Loading

kmilo9999 commented Jun 20, 2024 •

edited

Loading

ntolley left a comment •

edited

Loading