Stop F2PY caching #127
Stop F2PY caching #127
Conversation
The version in rangen.fpp which calls rm48 is used in the existing bindings, keeping this addtionally causes f2py to create two bindings to the same function, which is an error.
|
Thank you for your help, we appreciate it very much, I am currently traveling, but I will try to check this later today. |
|
@HaoZeke We don't want to touch epos-random.f, because it is external code that we just vendored in. I use epos-random-dummy.f instead to generate the interface, which does not contain rm48 or rmmard, so it should be the same fix as 2e3a0b8. I further modified our F2PY.cmake so that it does generate the cache files anymore. I copied again the f2py folder from numpy/numpy#22657 into my local numpy installation 1.23.5 and tried to build with With this setup, the module If you could spend some more time helping to debug this, I would be very grateful. You can restrict this project to only build the offending phojet191 module by generating a file called (module name without leading underscore). This helps to speed up the development. If you want to go back to building everything, just delete To see the log from running f2py, do |
|
Here is the log from my machine for reference. |
|
To confirm that this branch still works with f2py from numpy-v1.19.5, I deleted the f2py folder again in my local numpy installation and copied that from numpy-v1.19.5. I then deleted the You can optionally test whether the code not only compiles but also runs correctly with this command I don't think that the run-test is needed, but I wanted to mention it just in case. |
|
For the record, one can install the @HaoZeke branch with If someone knows to use such a URL in pyproject.toml for |
|
As another test, I now used |
The version in rangen.fpp which calls rm48 is used in the existing bindings, keeping this addtionally causes f2py to create two bindings to the same function, which is an error.
|
@HDembinski could you enable the workflow for this? I added a dependency on my branch with the fix in numpy/numpy#22657. Locally I'm able to build it, and run the test in the |
|
Thank you very much, I started the workflow. |
|
The failure is in the installation step for setuptools, we need to fix this before we can test the patch by @HaoZeke |
Sorry I had a typo in my last commit, could you try running it now? |
|
@HaoZeke It is not failing because the directory src/f2py has been removed. Let me fix that in the cmake scripts. |
|
@HaoZeke I fixed the main build issue. The macos-latest test fails, because some dependency (numpy? setuptools?) wants Python-3.9 as the minimum. I would like to keep 3.8 as the minimum for chromo, but this is something we don't have to worry about here for now. The crackfortran code now seems to fail in chromo/src/fortran/urqmd-3.4/sources/make22.f:5976 and following, see |
Makes sense, yup.
I'll look into this, I can reproduce this error with the |
|
@HaoZeke, I’ll look into this. It could be rather an issue with this legacy code. Edit: I meant, this file shouldn’t be in the list those to be parsed. |
|
Hello @HaoZeke, we have an issue with f2py again. All models fails to produce f2py for a recent release of numpy==1.26.0. Everything worked for numpy==1.26.0rc1. The simplest model Other models also have the similar problems related to DATA name_of_data /some_value/For sophia it is : DATA pi /3.141593D0/Some models ( File "/hetghome/antonpr/miniconda3/envs/env_test/lib/python3.9/site-packages/numpy/f2py/crackfortran.py", line 1471, in analyzeline
vars[v]['='] = matches[idx]
IndexError: list index out of rangeIt seems all the problems are related to recently changed code in numpy/numpy@0df1365 , specifically in the file vtype = vars[v].get('typespec')
vdim = getdimension(vars[v])
if (vtype == 'complex'):
cmplxpat = r"\(.*?\)"
matches = re.findall(cmplxpat, l[1])
else:
matches = l[1].split(',')
if v not in vars:
vars[v] = {}
if '=' in vars[v] and not vars[v]['='] == matches[idx]:
outmess('analyzeline: changing init expression of "%s" ("%s") to "%s"\n' % (
v, vars[v]['='], matches[idx]))
if vdim is not None:
# Need to assign multiple values to one variable
vars[v]['='] = "(/{}/)".format(", ".join(matches))
else:
vars[v]['='] = matches[idx]
last_name = vShould I add a new issue for numpy to fix this? |
Ouch, sorry, yes please open an issue and I'll get to it ASAP. |
@HaoZeke, issue is opened numpy/numpy#24746 |
@charris, in relation to backport, we encountered the following problem. We started to get reports from users about the error The comment in https://github.com/numpy/numpy/blob/maintenance/1.26.x/numpy/core/setup_common.py suggests:
However, to use bugfix in f2py we can build We didn't noticed the problem with API, because when we build wheels locally we don't have this problem, i.e. if our package is built locally with numpy==1.26.0 it still can be used with any numpy version > 1.19.5. However, wheels build on CI using |
That's odd, sounds like some sort of bug. How do you build locally? |
@charris, the initial steps in both CI and local builds are the same:
/hetghome/antonpr/miniconda3/envs/env_test/bin/gfortran -fPIC -O3 -shared -o /hetghome/antonpr/chromo/build/lib.linux-x86_64-cpython-39/chromo/models/_sib21.cpython-39-x86_64-linux-gnu.so CMakeFiles/_sib21.dir/fortranobject.c.o CMakeFiles/_sib21.dir/_sib21module.c.o "CMakeFiles/_sib21.dir/_sib21-f2pywrappers.f.o" CMakeFiles/_sib21.dir/src/fortran/sibyll/sibyll_21.f.o CMakeFiles/_sib21.dir/src/fortran/sibyll/sib21aux.f.o CMakeFiles/_sib21.dir/src/fortran/sibyll/sibyll_init.fpp.o CMakeFiles/_sib21.dir/src/fortran/logging.f.o CMakeFiles/_sib21.dir/src/fortran/rangen.fpp.o CMakeFiles/_sib21.dir/src/fortran/rangen.c.o CMakeFiles/_sib21.dir/src/fortran/normal.c.oOf course, it is all done with CMake. Another point which maybe relevant is that the compilation occurs with the flag "NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION". The difference between CI and local build is that CI uses cibuildwheel which fixes extension files to be dependent on general dynamical libraries, like, for example, on libraries contained in manylinux. So do you expect that the wheels build with numpy==1.26 should not work with numpy < 1.25? Or, on opposite, they should? |
They should. The versioning is based on the hash of the C API and 1,.25 and 1.26 have the same hash. |
@charris, it is clear about 1.25. What about lower version? Should the package work with numpy==1.20 to numpy=1.24 if it is built with numpy==1.26? |
In general, no. The hash changes when new features are added to the API, so any code that makes use of those features will fail on earlier versions. We try to keep forward compatibility, not backward compatibility. If you are making wheels for distribution, you should compile against the earliest version of NumPy that supports what you need. |
This is a PR towards chromo's 0.4 release. I hope it should be quick. There are a couple of issues. * The biggest current issue is #127. It is a problem for the source code, but wheels should work. This should be ignored for this release. * There is a small bug #163 which I will fix. * Licenses: ``` DPMJet/PhoJet -> Anatoli Fedynitch EPOS -> Tanguy Pierog and Klaus Werner (permission of usage) Sibyll -> Felix Riehn (permission of usage) QGSJet -> Sergei Ostapchenko (permission of usage) UrQMD-3.4 - ? Sophia - ? Pythia6 - ? Pythia8 - GPL2.0 ``` Anatoli suggested to put 3 permissions to corresponding source code directories. I guess `license.txt` with something like: ``` Sibyll- Redistribution Notice This software is distributed as part of "Chromo" with the permission of the original author. Please refer to the original license for terms and conditions. No warranty provided. ``` should be fine. What else should be done for release? --------- Co-authored-by: Hans Dembinski <hans.dembinski@gmail.com> Co-authored-by: afedynitch <afedynitch@gmail.com>
@HDembinski could you try building
numpywith numpy/numpy#22657 and running this branch? It should pass. Closes #117.