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

Inconsistent f2py wrappers #681

Closed
ketch opened this issue Apr 13, 2022 · 16 comments
Closed

Inconsistent f2py wrappers #681

ketch opened this issue Apr 13, 2022 · 16 comments

Comments

@ketch
Copy link
Member

ketch commented Apr 13, 2022

This is an issue that has been encountered by myself and @carlosmunozmoncayo on different machines.

Doing

import clawpack.pyclaw.classic.classic1 as c
c.step1?

at an IPython prompt shows the signature for the wrapper of the step1 function generated by f2py, which should be

q,cfl = step1(num_ghost,mx,q,aux,dx,dt,method,mthlim,use_fwave,rp1,[num_eqn,num_waves,num_aux,f,wave,s,amdq,apdq,dtdx,rp1_extra_args])

However, on the machines in question, it is instead

q,cfl = step1(q,aux,dx,dt,method,mthlim,use_fwave,rp1,[num_eqn,num_waves,num_ghost,num_aux,mx,f,wave,s,amdq,apdq,dtdx,rp1_extra_args])

In the latter case, f2py is inferring the values of num_ghost and mx instead of requiring that they be passed. This would be fine, but it breaks PyClaw since the signature of step1 is now different and the way it is called from Python is wrong.

I have experimented with different versions of Python, numpy, and gfortran, but haven't yet found what causes this problem. Has anyone else seen it?

@ketch
Copy link
Member Author

ketch commented Apr 13, 2022

For reference, the errors you will get at run time because of this are things like this:

ValueError: 0-th dimension must be 7 but got 0 (not defined).


The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/ketch/Research/Software/clawpack/pyclaw/examples/shallow_1d/dam_break.py", line 149, in <module>
    output = run_app_from_main(setup,setplot)
  File "/Users/ketch/Research/Software/clawpack/pyclaw/src/pyclaw/util.py", line 123, in run_app_from_main
    status = claw.run()
  File "/Users/ketch/Research/Software/clawpack/pyclaw/src/pyclaw/controller.py", line 362, in run
    status = self.solver.evolve_to_time(self.solution,t)
  File "/Users/ketch/Research/Software/clawpack/pyclaw/src/pyclaw/solver.py", line 616, in evolve_to_time
    self.step(solution, take_one_step, tstart, tend)
  File "/Users/ketch/Research/Software/clawpack/pyclaw/src/pyclaw/classic/solver.py", line 131, in step
    self.step_hyperbolic(solution)
  File "/Users/ketch/Research/Software/clawpack/pyclaw/src/pyclaw/classic/solver.py", line 304, in step_hyperbolic
    self.qbc,cfl = self.fmod.step1(num_ghost,mx,self.qbc,self.auxbc,dx,dt,self._method,self._mthlim,self.fwave,rp1)
ValueError: failed in converting 5th argument `method' of classic1.step1 to C/Fortran array

@ketch
Copy link
Member Author

ketch commented Apr 13, 2022

@mandli @rjleveque please let me know if you've seen this issue before.

@mandli
Copy link
Member

mandli commented Apr 13, 2022

Yup, I have. Not sure how I got around it now off-hand as I have not seen it in awhile.

@ketch
Copy link
Member Author

ketch commented Apr 14, 2022

Perhaps it would be worthwhile to "fix" this by inserting try-except blocks around the relevant function calls, with the alternative function signature in the except block. It's not pretty but it would solve this problem, which I suspect has affected others who we're not aware of.

@mandli
Copy link
Member

mandli commented Apr 14, 2022

Not ideal for sure. Have you seen the issue lately? It would be good to have some case that is reproducible.

@rjleveque
Copy link
Member

I haven't seen it, but have not been importing pyclaw solvers recently.

@ketch
Copy link
Member Author

ketch commented Apr 15, 2022

I currently have this issue on two machines. Both are macbooks, but they differ in a lot of other ways. One is running Mojave; the other Big Sur. We've tried different Python versions 3.7-3.10 with no change. Different versions of Gfortran. Different versions of numpy (including the latest release).

We've reached the point of patching it locally because we haven't figured out what causes it or how to fix it.

@ketch
Copy link
Member Author

ketch commented Apr 19, 2022

This seems to be a bug due to fairly recent changes in numpy. Rolling back to Numpy 1.18.5 fixed it for me. I've opened an issue in the numpy repository.

@mandli
Copy link
Member

mandli commented Apr 19, 2022

Assuming you meant numpy, my version is 1.21.5 and passes all nosetests except for the test tolerance problem.

@ketch
Copy link
Member Author

ketch commented Apr 19, 2022

LOL, yes I did not mean to suggest rolling back to Python 1!

@ketch
Copy link
Member Author

ketch commented Apr 19, 2022

And using a recent version of numpy seems to be a necessary but not sufficient condition for triggering the bug.

@HaoZeke
Copy link

HaoZeke commented Apr 28, 2022

I can't reproduce this on NumPy 1.23.

Help on module clawpack.pyclaw.classic.classic1 in clawpack.pyclaw.classic:

NAME
    clawpack.pyclaw.classic.classic1

DESCRIPTION
    This module 'classic1' is auto-generated with f2py (version:1.23.0.dev0+1087.g0eaa40db3).
    Functions:
        limiter(maxm,num_ghost,mx,wave,s,mthlim,num_eqn=shape(wave, 0),num_waves=shape(wave, 1))
        philim = philim(a,b,meth)
        q,cfl = step1(num_ghost,mx,q,aux,dx,dt,method,mthlim,use_fwave,rp1,num_eqn=shape(q, 0),num_waves=shape(mthlim, 0),num_aux=shape(aux, 0),f=,wave=,s=,amdq=,apdq=,dtdx=,rp1_extra_args=())
    .

DATA
    __f2py_numpy_version__ = '1.23.0.dev0+1087.g0eaa40db3'
    limiter = <fortran object>
    philim = <fortran philim>
    step1 = <fortran object>

VERSION
    1.23.0.dev0+1087.g0eaa40db3

@ketch
Copy link
Member Author

ketch commented Apr 28, 2022

In our experience the bug is not consistent. The function signature produced by f2py is different on different machines. But using numpy 1.18 always fixes it, and we've found no other way to fix it.

@jbarrios94
Copy link

jbarrios94 commented Apr 29, 2022

Hello, recently I was working with pyclaw and I got the same problem, what I did to solve it was to download the complete clawpack (.tar for example) copy the files step1.f90, step2.f90 (and others) in the pyclaw/classic folder and recompile them with the setup.py, that step will generate the files classic1, classic2, classic3 in the build folder, then replace them and the problem is solved.

@mandli
Copy link
Member

mandli commented Apr 29, 2022

This seems so odd with the version differences mattering. I do not see anything off-hand that would suggest that the parsing in f2py would have caused something different. Isn't there a way to explicitly declare the interfaces with f2py? Maybe we can use that to enforce what it should be?

@ketch
Copy link
Member Author

ketch commented May 7, 2022

After more investigation, it seems that this bug is already fixed in the current dev version of numpy. The bug exists only in the 1.22.x releases. We might want to add a warning on the Clawpack installation pages, advising people to avoid using numpy 1.22.x. Hopefully 1.23 will be released very soon.

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

No branches or pull requests

5 participants