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

Resolving environment file conda selector syntax #1603

Merged
merged 11 commits into from
Jul 22, 2024
Merged

Conversation

VeckoTheGecko
Copy link
Contributor

Making MPI explicitly an optional dependency that users need to install will simplify dev installation, and mean we don't have to worry about platform specific installation.

Changes:

  • Remove MPI packages from dev installation (ie, environment.yml)
  • Update install instructions
  • Update MPI documentation notebook
  • Remove mamba from GHA testing to reflect new installation instructions

Supercedes #1471 and #1513

Copy link
Contributor Author

@VeckoTheGecko VeckoTheGecko left a comment

Choose a reason for hiding this comment

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

@erikvansebille I was taking a look at the environment.yml file, and saw that there are doc building packages already included in there which means that docs/environment.yml is redundant. 2f2e4f7 implements your changes from #1513 to fix this

Co-authored-by: Erik van Sebille <e.vansebille@uu.nl>
@VeckoTheGecko
Copy link
Contributor Author

VeckoTheGecko commented Jul 17, 2024

Resolved the main test failures, but still getting a failure for the following 2 tests unrelated to MPI. Any insight @erikvansebille ? Perhaps its due to the switch from mamba to conda, and we might need to compare the dependency list before and after.

Logs:

unit tests

=========================== short test summary info ============================
FAILED tests/test_fieldset_sampling.py::test_fieldset_sample - assert False
 +  where False = <function allclose at 0x106139bb0>(array([-170.       , -167.14285  , -164.28572  , -161.42857  ,\n       -158.57141  , -155.7143   , -152.85715  , -150.       ,\n       -147.14285  , -144.28572  , -141.42857  , -138.57143  ,\n       -135.71428  , -132.85715  , -130.       , -127.14285  ,\n       -124.28571  , -121.42857  , -118.57143  , -115.71429  ,\n       -112.85715  , -110.       , -107.14286  , -104.28571  ,\n       -101.42857  ,  -98.57143  ,  -95.714294 ,  -92.85714  ,\n        -90.       ,  -87.14286  ,  -84.28571  ,  -81.42857  ,\n        -78.57143  ,  -75.71429  ,  -72.85714  ,  -70.       ,\n        -67.14286  ,  -64.28571  ,  -61.428574 ,  -58.57143  ,\n        -55.714287 ,  -52.857143 ,  -50.       ,  -47.142853 ,\n        -44.285717 ,  -41.42857  ,  -38.57143  ,  -35.714287 ,\n        -32.857143 ,  -30.       ,  -27.142857 ,  -24.285717 ,\n        -21.428572 ,  -18.571426 ,  -15.714286 ,  -12.8571415,\n        -10.       ,   -7.142857 ,   -4.285714 ,   -1.4285715,\n          1.4285715,    4.285714 ,    7.142857 ,   10.       ,\n         12.8571415,   15.714285 ,   18.571428 ,   21.428572 ,\n         24.285715 ,   27.142857 ,   30.       ,   32.857143 ,\n         35.714287 ,   38.57143  ,   41.42857  ,   44.285713 ,\n         47.142857 ,   50.       ,   52.857143 ,   55.714287 ,\n         58.57143  ,   61.428574 ,   64.28571  ,   67.14286  ,\n         70.       ,   72.85714  ,   75.71429  ,   78.57143  ,\n         81.42857  ,   84.28571  ,   87.14286  ,   90.       ,\n         92.85714  ,   95.714294 ,   98.57143  ,  101.42857  ,\n        104.285706 ,  107.14286  ,  110.       ,  112.85715  ,\n        115.71429  ,  118.57143  ,  121.42857  ,  124.28571  ,\n        127.14286  ,  130.       ,  132.85715  ,  135.71428  ,\n        138.57143  ,  141.42857  ,  144.28572  ,  147.14285  ,\n        150.       ,  152.85715  ,  155.71428  ,  158.57141  ,\n        161.42857  ,  164.28572  ,  167.14285  ,  170.       ],\n      dtype=float32), array([-170.       , -167.14285  , -164.28572  , -161.42857  ,\n       -158.57143  , -155.71428  , -152.85715  , -150.       ,\n       -147.14285  , -144.28572  , -141.42857  , -138.57143  ,\n       -135.71428  , -132.85715  , -130.       , -127.14286  ,\n       -124.28571  , -121.42857  , -118.57143  , -115.71429  ,\n       -112.85714  , -110.       , -107.14286  , -104.28571  ,\n       -101.42857  ,  -98.57143  ,  -95.71429  ,  -92.85714  ,\n        -90.       ,  -87.14286  ,  -84.28571  ,  -81.42857  ,\n        -78.57143  ,  -75.71429  ,  -72.85714  ,  -70.       ,\n        -67.14286  ,  -64.28571  ,  -61.42857  ,  -58.57143  ,\n        -55.714287 ,  -52.857143 ,  -50.       ,  -47.142857 ,\n        -44.285713 ,  -41.42857  ,  -38.57143  ,  -35.714287 ,\n        -32.857143 ,  -30.       ,  -27.142857 ,  -24.285715 ,\n        -21.428572 ,  -18.571428 ,  -15.714286 ,  -12.857142 ,\n        -10.       ,   -7.142857 ,   -4.285714 ,   -1.4285715,\n          1.4285715,    4.285714 ,    7.142857 ,   10.       ,\n         12.857142 ,   15.714286 ,   18.571428 ,   21.428572 ,\n         24.285715 ,   27.142857 ,   30.       ,   32.857143 ,\n         35.714287 ,   38.57143  ,   41.42857  ,   44.285713 ,\n         47.142857 ,   50.       ,   52.857143 ,   55.714287 ,\n         58.57143  ,   61.42857  ,   64.28571  ,   67.14286  ,\n         70.       ,   72.85714  ,   75.71429  ,   78.57143  ,\n         81.42857  ,   84.28571  ,   87.14286  ,   90.       ,\n         92.85714  ,   95.71429  ,   98.57143  ,  101.42857  ,\n        104.28571  ,  107.14286  ,  110.       ,  112.85714  ,\n        115.71429  ,  118.57143  ,  121.42857  ,  124.28571  ,\n        127.14286  ,  130.       ,  132.85715  ,  135.71428  ,\n        138.57143  ,  141.42857  ,  144.28572  ,  147.14285  ,\n        150.       ,  152.85715  ,  155.71428  ,  158.57143  ,\n        161.42857  ,  164.28572  ,  167.14285  ,  170.       ],\n      dtype=float32), rtol=1e-07)
 +    where <function allclose at 0x106139bb0> = np.allclose
FAILED tests/test_fieldset_sampling.py::test_fieldset_sample_eval - assert False
 +  where False = <function allclose at 0x106139bb0>(array([-80.       , -77.28814  , -74.57627  , -71.86441  , -69.15254  ,\n       -66.44068  , -63.728813 , -61.016953 , -58.30508  , -55.59322  ,\n       -52.881355 , -50.16949  , -47.457626 , -44.74576  , -42.033897 ,\n       -39.322037 , -36.61017  , -33.898304 , -31.18644  , -28.474577 ,\n       -25.762714 , -23.05085  , -20.338985 , -17.62712  , -14.915254 ,\n       -12.20339  ,  -9.491525 ,  -6.7796607,  -4.067797 ,  -1.3559322,\n         1.3559322,   4.0677967,   6.7796607,   9.491526 ,  12.20339  ,\n        14.915255 ,  17.62712  ,  20.338984 ,  23.050848 ,  25.762714 ,\n        28.474575 ,  31.186441 ,  33.898304 ,  36.61017  ,  39.322033 ,\n        42.033897 ,  44.745758 ,  47.457626 ,  50.169487 ,  52.881355 ,\n        55.59322  ,  58.305084 ,  61.01695  ,  63.728813 ,  66.44068  ,\n        69.15254  ,  71.86441  ,  74.57627  ,  77.28814  ,  80.       ],\n      dtype=float32), array([-80.       , -77.28814  , -74.57627  , -71.86441  , -69.15254  ,\n       -66.44068  , -63.728813 , -61.01695  , -58.305084 , -55.59322  ,\n       -52.881355 , -50.16949  , -47.457626 , -44.74576  , -42.033897 ,\n       -39.322033 , -36.61017  , -33.898304 , -31.186441 , -28.474577 ,\n       -25.762712 , -23.050848 , -20.338984 , -17.62712  , -14.915255 ,\n       -12.20339  ,  -9.491526 ,  -6.779661 ,  -4.0677967,  -1.3559322,\n         1.3559322,   4.0677967,   6.779661 ,   9.491526 ,  12.20339  ,\n        14.915255 ,  17.62712  ,  20.338984 ,  23.050848 ,  25.762712 ,\n        28.474577 ,  31.186441 ,  33.898304 ,  36.61017  ,  39.322033 ,\n        42.033897 ,  44.74576  ,  47.457626 ,  50.16949  ,  52.881355 ,\n        55.59322  ,  58.305084 ,  61.01695  ,  63.728813 ,  66.44068  ,\n        69.15254  ,  71.86441  ,  74.57627  ,  77.28814  ,  80.       ],\n      dtype=float32), rtol=1e-07)
 +    where <function allclose at 0x106139bb0> = np.allclose
= 2 failed, 962 passed, 1 skipped, 8 xfailed, 292 warnings in 655.66s (0:10:55) =

integration tests

=========================== short test summary info ============================
FAILED docs/examples/example_peninsula.py::test_peninsula_fieldset_AnalyticalAdvection[flat-scipy] - assert np.False_
 +  where np.False_ = <built-in method all of numpy.ndarray object at 0x7fddfe59a9d0>()
 +    where <built-in method all of numpy.ndarray object at 0x7fddfe59a9d0> = array([0.22306108, 0.09739923, 0.00405693, 0.04935837, 0.04673195,\n       0.00358963, 0.02781296, 0.01982117, 0.01034927, 0.01644897],\n      dtype=float32) <= 0.1.all
FAILED docs/examples/example_peninsula.py::test_peninsula_fieldset_AnalyticalAdvection[spherical-scipy] - assert np.False_
 +  where np.False_ = <built-in method all of numpy.ndarray object at 0x7fddfe5defd0>()
 +    where <built-in method all of numpy.ndarray object at 0x7fddfe5defd0> = array([0.22306108, 0.09739923, 0.00405693, 0.04935837, 0.04673195,\n       0.00358963, 0.02781296, 0.01982117, 0.01034927, 0.01644897],\n      dtype=float32) <= 0.1.all
= 2 failed, 329 passed, 112 deselected, 4 xfailed, 210 warnings in 934.38s (0:15:34) =

@VeckoTheGecko
Copy link
Contributor Author

VeckoTheGecko commented Jul 18, 2024

I did some testing, and those tests were also failing on master in my local development. Diffing the environment files, numpy 1.26.4 was working but numpy 2.0.0 (!! new major version!) was breaking. We can look at migration down the line, but for now let's just update the env file. This should propagate through to the recipe lets change test cases

NumPy 2.0.0 Release Notes numpy/numpy#24300

@VeckoTheGecko
Copy link
Contributor Author

VeckoTheGecko commented Jul 18, 2024

Doc build failure is due to the https://www.aoml.noaa.gov/phod/argo/images/argo_float_mission.jpg link not working (https://www.aoml.noaa.gov is down). It will likely come back up in a few days, so nothing of concern I think. Happy to merge pending your PR approval (and that the tolerances are ok) @erikvansebille

@VeckoTheGecko
Copy link
Contributor Author

VeckoTheGecko commented Jul 19, 2024

Increased error in the test_fieldset_sampling.py::test_fieldset_sample and test_fieldset_sampling.py::test_fieldset_sample_eval unit tests are due to changes in the grid interpolation. Likely due to numpy2's changes in how they related to float32 type promotion.

@VeckoTheGecko
Copy link
Contributor Author

VeckoTheGecko commented Jul 22, 2024

@erikvansebille Would you be able to merge here? I don't think I can due to CI failure/branch protection rules, and I don't have access to the settings tab for this repo :)
I think my permissions need to be modified

@VeckoTheGecko VeckoTheGecko merged commit 85f52df into master Jul 22, 2024
9 of 10 checks passed
@VeckoTheGecko VeckoTheGecko deleted the mpi-dep-changes branch July 22, 2024 07:15
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.

2 participants