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

Nightly wheels are out of date ? #133

Closed
neutrinoceros opened this issue Mar 2, 2024 · 7 comments · Fixed by #137
Closed

Nightly wheels are out of date ? #133

neutrinoceros opened this issue Mar 2, 2024 · 7 comments · Fixed by #137

Comments

@neutrinoceros
Copy link
Contributor

Astropy's devdeps tests are currently crashing at startup

For instance, see https://github.com/astropy/astropy/actions/runs/8119881813/job/22196363368?pr=16087

...
packages/erfa/__init__.py", line 5, in <module>
    from .version import version as __version__  # noqa
  File "/home/runner/work/astropy/astropy/.tox/py312-test-devdeps/lib/python3.12/site-packages/erfa/version.py", line 25, in <module>
    from . import ufunc
RuntimeError: module compiled against ABI version 0x1000009 but this version of numpy is 0x2000000

From what I can see, my guess is that the latest nightlies available at https://pypi.anaconda.org/liberfa/simple are out of date and not compatible with the numpy's latest nightly builds. To be more exact, at the time of writing, numpy nightlies were last uploaded 17 hours ago (see https://github.com/numpy/numpy/actions/workflows/wheels.yml), but pyerfa's last successful upload is from 29 hours ago (see https://github.com/liberfa/pyerfa/actions).
This would normally not be a problem since new nightlies are supposed to be uploaded well, nightly. However it looks like the automation is currently flaky and failing more often than not (see https://github.com/liberfa/pyerfa/actions). Specifically, Windows builds are failing, and it seems that nothing gets uploaded unless every build succeeds. I'd suggest changing this last rule as a quick way to mitigate the problem (I suspect most nightly users only care about Linux wheels anyway). I'm happy to open a PR on Monday if that is still relevant (there are 2 other scheduled uploads between now and then, a single succesful one would resolve the issue downstream, as far as I can tell).

I have not researched the reason why windows builds are flaky (or non deterministic, for what it's worth), but I'm happy to give a hand in solving the root problem if I can. Let me know !

@neutrinoceros
Copy link
Contributor Author

Note that, in the above, I made the assumption that the latest successful build correctly used the bleeding edge numpy version at the time it was produced, and just happened to be invalidated a couple hours later by another ABI change. However, the complete error message seems to indicate that pyerfa was actually built against numpy 1.x
One of these (my assumption or the error message) is wrong, I just don't know which.

@MridulS
Copy link

MridulS commented Mar 2, 2024

The flaky test here is the test that passed https://github.com/liberfa/pyerfa/actions/runs/8106442866!! It just happen to pass because it got numpy 1.X while numpy wheels disappeared from the nightly channel numpy/numpy#25907

@neutrinoceros
Copy link
Contributor Author

Wow. Thanks for figuring it out !
I think we should dedicate some effort into fixing compat with numpy 2 now.

@neutrinoceros
Copy link
Contributor Author

Since I don't have direct access to a windows machine, I've hacked a simple GHA workflow on my fork to produce and upload generated ufunc.c with numpy stable and numpy dev. See https://github.com/neutrinoceros/pyerfa/actions/runs/8129786249
Perhaps unsurprisingly, both ufunc.c artifacts are actually identical. But still, good to know. I'll dig further in a couple hours.

@neutrinoceros
Copy link
Contributor Author

neutrinoceros commented Mar 3, 2024

So the error we get (twice) is:

  erfa\ufunc.c(9714): error C2057: expected constant expression
  erfa\ufunc.c(9714): error C2466: cannot allocate an array of constant size 0
  erfa\ufunc.c(9714): error C2133: 'types_array': unknown size

which point to this line:

    int types_array[NPY_MAXARGS];

The issue here is that NPY_MAXARGS is now runtime dependent (32 with numpy 1.x, 64 with numpy 2) (see https://numpy.org/devdocs/reference/c-api/array.html#c.NPY_MAXARGS), but using it as an array size requires that its value be known at compile time, hence the compilation error.

I further note that the migration guide explicitly discourages use of this constant
https://numpy.org/devdocs/numpy_2_0_migration_guide.html#increased-maximum-number-of-dimensions
What's not yet clear to me is wether we can just drop it and why it's only showing up on Windows, but we're getting close !

@neutrinoceros
Copy link
Contributor Author

The following crude patch seems to resolve the problem:

diff --git a/erfa/ufunc.c.templ b/erfa/ufunc.c.templ
index a904622..2fc2e79 100644
--- a/erfa/ufunc.c.templ
+++ b/erfa/ufunc.c.templ
@@ -572,7 +572,8 @@ static int ErfaUFuncTypeResolver(PyUFuncObject *ufunc,
 {
     int *types;
     PyArray_Descr **dtypes;
-    int types_array[NPY_MAXARGS];
+    assert( NPY_MAXARGS <= 64 );
+    int types_array[64];
 
     if (ufunc->userloops) {
         Py_ssize_t unused_pos = 0;
@@ -752,7 +753,9 @@ PyMODINIT_FUNC PyInit_ufunc(void)
     PyArray_Descr *dt_ymdf = NULL, *dt_hmsf = NULL, *dt_dmsf = NULL;
     PyArray_Descr *dt_sign = NULL, *dt_type = NULL;
     PyArray_Descr *dt_eraASTROM = NULL, *dt_eraLDBODY = NULL;
-    PyArray_Descr *dtypes[NPY_MAXARGS];
+
+    assert( NPY_MAXARGS <= 64 );
+    PyArray_Descr *dtypes[64];
     /* ufuncs and their definitions */
     int status;
     PyUFuncObject *ufunc;

I'm using assert macros as a poorman's future proofing, maybe there's a better solution. I'll open a PR to specifically discuss this patch.

@mhvk
Copy link
Contributor

mhvk commented Mar 3, 2024

I think there is an additional problem that we're still compiling against numpy 1.x. Just removing the version constraint in #122 was not enough. I'm investigating...

@mhvk mhvk mentioned this issue Mar 3, 2024
@mhvk mhvk closed this as completed in #137 Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants