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

ADflow doesn't work with numpy 1.24 #256

Closed
A-CGray opened this issue Jan 23, 2023 · 4 comments · Fixed by #281
Closed

ADflow doesn't work with numpy 1.24 #256

A-CGray opened this issue Jan 23, 2023 · 4 comments · Fixed by #281
Labels
bug Something isn't working

Comments

@A-CGray
Copy link
Member

A-CGray commented Jan 23, 2023

Description

Self explanatory. We are trying to update the "latest" docker images to numpy 1.24 (https://github.com/mdolab/docker/pull/208) and getting the error below in all the ADflow tests when calling setattr, my best guess is that something changed in f2py.

Steps to reproduce issue

  1. Install numpy v1.24
  2. Run ADflow tests

Current behavior

 (mpi) /home/***/repos/adflow/tests/unit_tests/test_restart.py:BasicTests.test_import_block_splitting ... FAIL (00:00:0.03, 159 MB)
Traceback (most recent call last):
  File "/home/***/.pyenv/versions/3.10.9/lib/python3.10/site-packages/testflo/test.py", line 418, in _try_call
    func()
  File "/home/***/repos/adflow/tests/unit_tests/test_restart.py", line 33, in test_import_block_splitting
    CFDSolver = ADFLOW(options=self.options, debug=False)
  File "/home/***/.pyenv/versions/3.10.9/lib/python3.10/site-packages/adflow/pyADflow.py", line 148, in __init__
    super().__init__(
  File "/home/***/.pyenv/versions/3.10.9/lib/python3.10/site-packages/baseclasses/solvers/pyAero_solver.py", line 45, in __init__
    super().__init__(
  File "/home/***/.pyenv/versions/3.10.9/lib/python3.10/site-packages/baseclasses/solvers/BaseSolver.py", line 86, in __init__
    self.setOption(key, optionValue)
  File "/home/***/.pyenv/versions/3.10.9/lib/python3.10/site-packages/adflow/pyADflow.py", line 5207, in setOption
    setattr(module, variable, value)
ValueError: 0-th dimension must be 256 but got 0 (not defined).

Expected behavior

All tests pass

@sseraj
Copy link
Collaborator

sseraj commented Feb 17, 2023

The errors in the issue description are caused by a bug in f2py that has recently been fixed in numpy/numpy#23194. I built numpy with the fix from source (1.25.0.dev0+678.g51ecf8401), recompiled ADflow, and all the tests passed. We just need to wait for the next patch release, which should include this fix. The same applies to pyHyp.

There is a separate bug that occurs in ADflow when updating from 1.21 to 1.22. This looks like the intended behavior from f2py related to intent(out) strings of undetermined length:

File "/home/mdolabuser/repos/adflow/adflow/pyADflow.py", line 5065, in setOption
    self.adflow.inputparamroutines.isovariables(varStr)
libadflow.error: try_pyarr_from_string failed

This bug is relatively easy to fix in ADflow, so I will open a PR to address that.

@sseraj sseraj mentioned this issue Feb 17, 2023
13 tasks
@A-CGray
Copy link
Member Author

A-CGray commented Feb 17, 2023

The errors in the issue description are caused by a bug in f2py that has recently been fixed in numpy/numpy#23194. I built numpy with the fix from source (1.25.0.dev0+678.g51ecf8401), recompiled ADflow, and all the tests passed. We just need to wait for the next patch release, which should include this fix. The same applies to pyHyp.

There is a separate bug that occurs in ADflow when updating from 1.21 to 1.22. This looks like the intended behavior from f2py related to intent(out) strings of undetermined length:

File "/home/mdolabuser/repos/adflow/adflow/pyADflow.py", line 5065, in setOption
    self.adflow.inputparamroutines.isovariables(varStr)
libadflow.error: try_pyarr_from_string failed

This bug is relatively easy to fix in ADflow, so I will open a PR to address that.

Awesome, nice work!

@anilyil
Copy link
Contributor

anilyil commented Mar 10, 2023

This issue is fixed, correct? @A-CGray @sseraj

@A-CGray
Copy link
Member Author

A-CGray commented Mar 11, 2023

I think when numpy 1.24.3 is released we should update the requirements in setup.py to reflect the versions of numpy that does work with. We can close this issue with that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants