Test scikit.odes installation nightly #2973

arjxn-py · 2023-05-20T15:16:44Z

Description

run_periodic_tests.yml will now run test for pybamm_install_odes on GNU/Linux

Fixes #2962

Type of change

New feature (non-breaking change which adds functionality)
Optimization (back-end change that speeds up the code)
Bug fix (non-breaking change which fixes an issue)

Key checklist:

No style issues: $ pre-commit run (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
All tests pass: $ python run-tests.py --all
The documentation builds: $ python run-tests.py --doctest

You can run unit and doctests together at once, using $ python run-tests.py --quick.

Further checks:

Code is commented, particularly in hard-to-understand areas
Tests added that prove fix is effective or that feature works

codecov · 2023-05-20T15:41:23Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (a0b5a4c) 99.71% compared to head (ffc0e6a) 99.71%.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop    #2973    +/-   ##
=========================================
  Coverage    99.71%   99.71%            
=========================================
  Files          273      245    -28     
  Lines        19002    18705   -297     
=========================================
- Hits         18947    18651   -296     
+ Misses          55       54     -1

see 36 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Saransh-cpp

Thanks, @arjxn-py!

Could you check if this test works by failing the workflows intentionally once? Maybe you can revert back SUNDIALS to an older version in install_odes.py and check if the workflow fails on your fork (workflow_dispatch is enabled; hence, you might see a run workflow button after selecting this workflow in the Actions tab of your fork).

.github/workflows/run_periodic_tests.yml

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

arjxn-py · 2023-05-22T17:51:34Z

Thanks, @arjxn-py!

Could you check if this test works by failing the workflows intentionally once? Maybe you can revert back SUNDIALS to an older version in install_odes.py and check if the workflow fails on your fork (workflow_dispatch is enabled; hence, you might see a run workflow button after selecting this workflow in the Actions tab of your fork).

I have tried doing that @Saransh-cpp in the develop branch of my fork, reverted back SUNDIALS to an older version & benchmarks one is still failing.

Saransh-cpp · 2023-05-23T06:36:49Z

@arjxn-py you should trigger the workflow on the Issue-#2962-add-testing-for-pybamm_install_odes branch of your fork. Revert SUNDIALS version in your branch -> Go to Actions -> Select Scheduled from the left pane -> You will see a "Run workflow" button -> Trigger it for your branch.

Something like this -

arjxn-py · 2023-05-23T06:51:11Z

@arjxn-py you should trigger the workflow on the Issue-#2962-add-testing-for-pybamm_install_odes branch of your fork. Revert SUNDIALS version in your branch -> Go to Actions -> Select Scheduled from the left pane -> You will see a "Run workflow" button -> Trigger it for your branch.

Apologies for the confusion, now doing as you suggested.

arjxn-py · 2023-05-26T15:16:23Z

@Saransh-cpp here are the test results

Saransh-cpp · 2023-05-26T17:21:45Z

I see the tests are failing, but not how we thought they would haha. Remember, the workflows use tox everywhere to run any kind of tests; hence, the step added by you errors out with -

/home/runner/work/_temp/a433f960-04d6-49e8-b22c-dc1934db5611.sh: line 1: pybamm_install_odes: command not found

because pybamm was never installed in the base environment.

Why don't we do something like this -

saransh@Saranshs-MacBook-Air PyBaMM % git diff
diff --git a/.github/workflows/run_periodic_tests.yml b/.github/workflows/run_periodic_tests.yml
diff --git a/.github/workflows/run_periodic_tests.yml b/.github/workflows/run_periodic_tests.yml
index 39ad33c05..cf73f4398 100644
--- a/.github/workflows/run_periodic_tests.yml
+++ b/.github/workflows/run_periodic_tests.yml
@@ -124,3 +124,7 @@ jobs:
       - name: Install dev dependencies and run example tests
         if: matrix.os == 'ubuntu-latest'
         run: tox -e examples
+
+      - name: Install scikits.odes and test pybamm_install_odes
+        if: matrix.os == 'ubuntu-latest'
+        run: tox -e odes
diff --git a/tox.ini b/tox.ini
index 784d15494..d145aae6a 100644
--- a/tox.ini
+++ b/tox.ini
@@ -23,6 +23,7 @@ deps =
 commands =
      tests-!windows-!mac: sh -c "pybamm_install_jax"  # install jax, jaxlib for ubuntu
      unit-!windows-!mac: sh -c "pybamm_install_jax"  # install jax, jaxlib for ubuntu
+     odes-!windows-!mac: sh -c "pybamm_install_odes"  # test install_odes.py
      tests: python run-tests.py --all
      unit: python run-tests.py --unit

This should theoretically work.

arjxn-py · 2023-05-27T16:40:45Z

I see the tests are failing, but not how we thought they would haha. Remember, the workflows use tox everywhere to run any kind of tests.

Thanks, it was much helpful.
I am running Scheduled tests again now.

arjxn-py · 2023-05-28T03:40:12Z

@Saransh-cpp I believe that the tests ran as expected this time.

Saransh-cpp

It still does not work. Remember, we are trying to reproduce a very particular error, and not any error. The error that the CI throws right now is an actual error in this PR -

odes run-test-pre: PYTHONHASHSEED='840055894'
odes run-test: commands[0] | sh -c pybamm_install_odes
/home/runner/work/PyBaMM/PyBaMM/pybamm/scikits_odes_setup.log
Traceback (most recent call last):
  File "/home/runner/work/PyBaMM/PyBaMM/.tox/odes/bin/pybamm_install_odes", line 33, in <module>
    sys.exit(load_entry_point('pybamm', 'console_scripts', 'pybamm_install_odes')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/PyBaMM/PyBaMM/pybamm/install_odes.py", line 130, in main
    default_install_dir = os.path.join(os.getenv("HOME"), ".local")
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen posixpath>", line 76, in join
TypeError: expected str, bytes or os.PathLike object, not NoneType
ERROR: InvocationError for command /usr/bin/sh -c pybamm_install_odes (exited with code 1)
___________________________________ summary ____________________________________
ERROR:   odes: commands failed

The script cannot find the HOME env variable. You might have to pass it manually in the tox environment,

.github/workflows/run_periodic_tests.yml

arjxn-py · 2023-05-28T12:48:36Z

@Saransh-cpp i've passed HOME env variable explicitely & now all tests are passing.

pybamm/install_odes.py

Saransh-cpp

Thanks, @arjxn-py! Working much better now. One last thing -

GH Actions is still installing scikits.odes through cache -

Collecting scikits.odes
  Using cached scikits.odes-2.7.0-cp310-cp310-linux_x86_64.whl
Requirement already satisfied: scipy in ./.tox/odes/lib/python3.10/site-packages (from scikits.odes) (1.10.1)
Requirement already satisfied: numpy<1.27.0,>=1.19.5 in ./.tox/odes/lib/python3.10/site-packages (from scipy->scikits.odes) (1.24.3)
Installing collected packages: scikits.odes
Successfully installed scikits.odes-2.7.0

I think removing the cache before executing pybamm_install_odes with -

pip cache remove scikits.odes

should work.

tox.ini

Saransh-cpp

The error is a good sign. Remember we decided to revert SUNDIALS version to check if the script fails and it did fail. We do have one last problem. The script does not return the correct error code even after failing, making the GH Actions pass, which should be failing. There was a similar issue with install_KLU_sundials.py, see #2971 for a quick fix.

brosaplanella · 2023-05-30T13:50:19Z

The error is a good sign. Remember we decided to revert SUNDIALS version to check if the script fails and it did fail. We do have one last problem. The script does not return the correct error code even after failing, making the GH Actions pass, which should be failing. There was a similar issue with install_KLU_sundials.py, see #2971 for a quick fix.

Could this be related to the error we have in the publish workflow? @Saransh-cpp

Saransh-cpp · 2023-05-30T13:58:09Z

@brosaplanella I've pushed fixes for macos and ubuntu here - https://github.com/pybamm-team/PyBaMM/tree/release-fix - with the workflow running here - https://github.com/pybamm-team/PyBaMM/actions/runs/5122444804

The macos fix works for me locally. Taking a look at the ubuntu and the windows fix.

arjxn-py · 2023-05-30T14:38:00Z

The error is a good sign. Remember we decided to revert SUNDIALS version to check if the script fails and it did fail. We do have one last problem. The script does not return the correct error code even after failing, making the GH Actions pass, which should be failing. There was a similar issue with install_KLU_sundials.py, see #2971 for a quick fix.

Thanks @Saransh-cpp, I had gone through #2971 & made some changes in accordance to it now i'm getting quite obvious failing logs, are these the one we expecting?

Warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]
      INFO: /usr/bin/gfortran -Wall -g -Wall -g -shared build/temp.linux-x86_64-cpython-311/build/src.linux-x86_64-3.11/scikits/odes/lsodimodule.o build/temp.linux-x86_64-cpython-311/build/src.linux-x86_64-3.11/build/src.linux-x86_64-3.11/scikits/odes/fortranobject.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/xerrwv.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/prepji.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/solsy.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/ainvg.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/ewset.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/ddaspk.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/cfode.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/dlinpk.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/intdy.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/daux.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/stodi.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/lsodi.o build/temp.linux-x86_64-cpython-311/scikits/odes/daepack/vnorm.o build/temp.linux-x86_64-cpython-311/build/src.linux-x86_64-3.11/scikits/odes/lsodi-f2pywrappers.o -L/usr/lib/gcc/x86_64-linux-gnu/11 -L/usr/lib/gcc/x86_64-linux-gnu/11 -L/usr/lib/x86_64-linux-gnu -lgfortran -o build/lib.linux-x86_64-cpython-311/scikits/odes/lsodi.cpython-311-x86_64-linux-gnu.so
      INFO: building 'scikits.odes.sundials.common_defs' extension
      INFO: compiling C sources
      INFO: C compiler: x86_64-linux-gnu-gcc -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -fPIC

      creating build/temp.linux-x86_64-cpython-311/scikits/odes/sundials
      INFO: compile options: '-I/home/arjxnpy/.local/include -I/tmp/pip-build-env-7bgtujw_/overlay/lib/python3.11/site-packages/numpy/core/include -Ibuild/src.linux-x86_64-3.11/numpy/distutils/include -I/mnt/c/Users/Arjun/Desktop/PyBaMM/.tox/odes/include -I/usr/include/python3.11 -c'
      extra options: '-msse -msse2 -msse3'
ERROR:   odes: commands failed

Saransh-cpp · 2023-05-31T05:06:29Z

@arjxn-py can you link the latest scheduled run on your fork?

arjxn-py · 2023-05-31T09:47:15Z

@arjxn-py can you link the latest scheduled run on your fork?

Sure @Saransh-cpp, here.

pybamm/install_odes.py

Saransh-cpp

Looks good, thanks for working on this patiently, @arjxn-py!

arjxn-py · 2023-05-31T18:48:19Z

Looks good, thanks for working on this patiently, @arjxn-py!

Thanks @Saransh-cpp, you're a big help.
I also ran tests to verify & they're all passing.

Test scikit.odes installation nightly

c6e1ddf

Saransh-cpp self-requested a review May 20, 2023 17:30

Saransh-cpp reviewed May 21, 2023

View reviewed changes

.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved

.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved

arjxn-py and others added 2 commits May 22, 2023 19:09

Update .github/workflows/run_periodic_tests.yml

00d49b6

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Add suggested change from review

d5b984a

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

arjxn-py requested a review from Saransh-cpp May 23, 2023 06:25

Revert back to sundials previous version

4149699

Test odes installation via tox

bb8e161

Saransh-cpp requested changes May 28, 2023

View reviewed changes

.github/workflows/run_periodic_tests.yml Outdated Show resolved Hide resolved

arjxn-py force-pushed the Issue-#2962-add-testing-for-pybamm_install_odes branch from a028241 to bb8e161 Compare May 28, 2023 09:26

pass HOME env variable explicitely

a54a482

Saransh-cpp reviewed May 28, 2023

View reviewed changes

pybamm/install_odes.py Outdated Show resolved Hide resolved

arjxn-py added 2 commits May 28, 2023 23:37

pass HOME env variable manually in tox

b77b53a

Remove pre-existing SUNDIALS & scikit.odes

34bab1c

arjxn-py requested a review from Saransh-cpp May 29, 2023 09:58

Saransh-cpp reviewed May 30, 2023

View reviewed changes

tox.ini Outdated Show resolved Hide resolved

arjxn-py added 2 commits May 30, 2023 16:29

Add comments for commands

0b7386b

Club commands in a single line

374c5b8

arjxn-py requested a review from Saransh-cpp May 30, 2023 13:20

Saransh-cpp reviewed May 30, 2023

View reviewed changes

arjxn-py and others added 2 commits May 30, 2023 20:00

Add checks for subprocesses

952294a

style: pre-commit fixes

2fb0ea7

Saransh-cpp reviewed May 31, 2023

View reviewed changes

pybamm/install_odes.py Outdated Show resolved Hide resolved

Revert to sundials 6.5.0

ffc0e6a

Saransh-cpp approved these changes May 31, 2023

View reviewed changes

valentinsulzer merged commit 08b9d5e into pybamm-team:develop Jun 12, 2023
17 of 19 checks passed

arjxn-py deleted the Issue-#2962-add-testing-for-pybamm_install_odes branch June 15, 2023 13:14

agriyakhetarpal mentioned this pull request Oct 7, 2023

Extend pybamm_install_odes to include support for macOS systems #3417

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test scikit.odes installation nightly #2973

Test scikit.odes installation nightly #2973

arjxn-py commented May 20, 2023 •

edited

Loading

codecov bot commented May 20, 2023 •

edited

Loading

Saransh-cpp left a comment

arjxn-py commented May 22, 2023

Saransh-cpp commented May 23, 2023

arjxn-py commented May 23, 2023

arjxn-py commented May 26, 2023

Saransh-cpp commented May 26, 2023

arjxn-py commented May 27, 2023 •

edited

Loading

arjxn-py commented May 28, 2023 •

edited

Loading

Saransh-cpp left a comment

arjxn-py commented May 28, 2023

Saransh-cpp left a comment

Saransh-cpp left a comment

brosaplanella commented May 30, 2023

Saransh-cpp commented May 30, 2023 •

edited

Loading

arjxn-py commented May 30, 2023 •

edited

Loading

Saransh-cpp commented May 31, 2023

arjxn-py commented May 31, 2023

Saransh-cpp left a comment

arjxn-py commented May 31, 2023

Test scikit.odes installation nightly #2973

Test scikit.odes installation nightly #2973

Conversation

arjxn-py commented May 20, 2023 • edited Loading

Description

Type of change

Key checklist:

Further checks:

codecov bot commented May 20, 2023 • edited Loading

Codecov Report

Saransh-cpp left a comment

Choose a reason for hiding this comment

arjxn-py commented May 22, 2023

Saransh-cpp commented May 23, 2023

arjxn-py commented May 23, 2023

arjxn-py commented May 26, 2023

Saransh-cpp commented May 26, 2023

arjxn-py commented May 27, 2023 • edited Loading

arjxn-py commented May 28, 2023 • edited Loading

Saransh-cpp left a comment

Choose a reason for hiding this comment

arjxn-py commented May 28, 2023

Saransh-cpp left a comment

Choose a reason for hiding this comment

Saransh-cpp left a comment

Choose a reason for hiding this comment

brosaplanella commented May 30, 2023

Saransh-cpp commented May 30, 2023 • edited Loading

arjxn-py commented May 30, 2023 • edited Loading

Saransh-cpp commented May 31, 2023

arjxn-py commented May 31, 2023

Saransh-cpp left a comment

Choose a reason for hiding this comment

arjxn-py commented May 31, 2023

arjxn-py commented May 20, 2023 •

edited

Loading

codecov bot commented May 20, 2023 •

edited

Loading

arjxn-py commented May 27, 2023 •

edited

Loading

arjxn-py commented May 28, 2023 •

edited

Loading

Saransh-cpp commented May 30, 2023 •

edited

Loading

arjxn-py commented May 30, 2023 •

edited

Loading