SciPy v1.13.0

[agriyakhetarpal]

Description

This PR bumps the in-tree SciPy version in the Pyodide distribution to v1.13.0. This is an in-progress change, as I look to fix some of the older patches locally, and therefore this PR is being opened for purposes of visibility only at this time.

cc: @rgommers, and the package maintainers @lesteve, @steppi as listed in the recipe.

To be done

• Fix patches and updates files that have been upstreamed
• Look at changes required scipy/_build_utils/src/wrap_g77_abi.c – changed from scipy/_build_utils/src/wrap_g77_abi.f in SciPy v1.12.0
• Adjust build scripts and compiler flags as necessary
• Scout for changes to the public API between these two versions and update imports:
• Ensure all tests and imports pass

Checklists

• Add a CHANGELOG entry
• Add / update tests
• Add new / update outdated documentation

[agriyakhetarpal]

I see that SciPy is being reverted from v1.13.0 to v1.12.0 in #4712 – I'm not sure what that means, so I'll be happy to wait until further progress or discussion on that PR (my understanding is that whichever packages still pass the tests with newer versions can stay and will not be reverted?)

[hoodmane]

I'm not sure what that means, so I'll be happy to wait until further progress or discussion on that PR (my understanding is that whichever packages still pass the tests with newer versions can stay and will not be reverted?)

We do one big PR for all the easy packages and then have to handle the ones that cause trouble one at a time as we have time. Please go ahead with working on this PR. If there is a conflict with #4712 for some reason I can help resolve it.

[lesteve]

@agriyakhetarpal just curious, what is the status of this? Since you mentioned "as as I look to fix some of the older patches locally" I am guessing that you are still working on updating the patches?

For some weird reasons, it looks like the full CI was not run (random guess maybe because this is a Draft PR, although seems unlikely), can you try pushing an empty commit to trigger the CI again? I was confused and thought this PR was green untill I unfolded the "All checks" and found out that there are only readthedocs and pre-commit ...

[agriyakhetarpal]

@agriyakhetarpal just curious, what is the status of this? Since you mentioned "as as I look to fix some of the older patches locally" I am guessing that you are still working on updating the patches?

Hi, @lesteve, thank you for the ping. I have just returned to this PR late last week, and I was going to post an update today about how this is going, so, yes, doing this and updating the patches here is well on my radar for this week. I'll be sure to request help wherever I don't understand something fully!

For some weird reasons, it looks like the full CI was not run (random guess maybe because this is a Draft PR, although seems unlikely), can you try pushing an empty commit to trigger the CI again? I was confused and thought this PR was green untill I unfolded the "All checks" and found out that there are only readthedocs and pre-commit ...

Ah, I guess the full CI was not run because I added the [skip ci] label in the commit message in order to save resources for others. :) I shall trigger CI in subsequent commits or when I rebase.

[lesteve]

Ah makes sense I missed the [skip ci] thing and thought "OK the CI seems green, so this should be good to merge"

[hoodmane]

It's an unfortunate thing about Skip Ci. Maybe we should add a "ci ran" check. Of course we can merge changes that skip CI, but the Ci is flaky enough that we merge failing CI a lot.

[agriyakhetarpal]

I rebased on top of the latest changes from the main branch and fixed up the patches (all that was to fix, most likely) – I will hope that CI passes (🤞), and I shall update the to-do list in the PR description accordingly.

[agriyakhetarpal]

I assume that the failures under ci/circleci: test-core-safari are unrelated, I will wait until the ci/circleci: build-libraries job passes (or runs till its completion).

[agriyakhetarpal]

Oops, it looks like scipy is under the "The following packages are disabled" list: https://app.circleci.com/pipelines/github/pyodide/pyodide/7181/workflows/88fa2666-a007-427a-826d-cd207b3a4485/jobs/100486?invite=true#step-104-9703_80, so it's not being built. I'll look if there is a special commit message to build SciPy or the SciPy stack in general, based on https://pyodide.org/en/stable/development/building-from-sources.html#partial-builds.

Edit: never mind, I saw that some new builds popped up on CircleCI.

[agriyakhetarpal]

Build logs: https://app.circleci.com/pipelines/github/pyodide/pyodide/7182/workflows/36dda168-fe41-4dcf-859d-278178308521/jobs/100512/parallel-runs/0/steps/0-104

Note to self: most of the errors look to be coming from Fortran code because the f2c sub-command failed; I have to look at syntax errors in a few files, remove warnings about a few compiler flags (probably replace -Wno-maybe-unintialized with -Wno-unintialized and look at how Emscripten sets runtime search paths because -rpath was reported to be unknown, etc.), and correctly identify the file leading to the cause of the error (I am currently browsing through the logs on a mobile device and that does not help) – it's most likely inside FITPACK.

[agriyakhetarpal]

It's an unfortunate thing about Skip Ci. Maybe we should add a "ci ran" check. Of course we can merge changes that skip CI, but the Ci is flaky enough that we merge failing CI a lot.

Btw, @hoodmane, if you could elaborate on this slightly, I would be happy to add this "ci ran" check in some shape or form (if the change would not be non-trivial, that is, of course).

[agriyakhetarpal]

The errors were in scipy/interpolate/fitpack/fpchec.f (sub-module 1076 out of 1464) and were coming from lines like these:

if(t(i).le.t(i-1))then; ier=30; go to 80; endif
...
if(x(1).lt.t(k1) .or. x(m).gt.t(nk2))then; ier=40; go to 80;
endif
...
if(i.ge.m)then; ier=50; go to 80; endif

and so on. I checked the file versus SciPy v1.12 and scipy/scipy@a6284f6.patch is where this comes from, so I switched the if-else constructs to multiple lines in 6917e33, and that makes them work for some reason.

and now SciPy v1.13.0 builds successfully (🎉): workflow logs. I'll wait for the build to finish, following which I can add a CHANGELOG entry when this will be merged (please let me know if I should skip CI for that one) and complete the rest of the checklists.

[agriyakhetarpal]

Never mind, there are 4 failures out of 5 tests on SciPy: ImportError: dynamic module does not define module export function (PyInit_cython_lapack)... I am diving further into this.

Edit: and similarly for PyInit__sparsetools

[agriyakhetarpal]

I don't entirely know how to debug this and I am afraid that my attempts to look further did not lead to much insight. As far as I understand, not being experienced with Fortran codes, there could be a missing PyInit_* statement in the scipy.linalg side of things, or something was missed during the compilation of cython_lapack inside scipy/linalg/meson.build – I notice that 1.12.0 used link_with: fwrappers and 1.13.0 uses link_with: [g77_abi_wrappers], and I tried to follow the discussions around that and I think the modified patch 5/8 should fix that in scipy/_build_utils/src/wrap_g77_abi.c. I inspected the auto-generated cython_lapack.c file diffs between v1.12.0 and v1.13.0 and I didn't catch anything relevant, except for the F_FUNC macros having been replaced by BLAS_FUNC everywhere, likely a result of the ABI wrappers.

Happy to receive suggestions on how I can debug this further and work towards a solution, @ryanking13 and @hoodmane – your comments will be highly appreciated!

[hoodmane]

I'll take a look when I have a minute. At pycon right now.

[rgommers]

If it helps, scipy/scipy#20232 should be the relevant SciPy PR I believe. It replaced some of the problematic Fortran wrappers for ABI differences with C wrappers.

[ryanking13]

Thanks for working on this! I will take a look after the PyCon (probably after May 23)

[agriyakhetarpal]

@rgommers, I had a question that is extrinsic to these changes but probably still relevant – is there a policy on upstreaming particular non-intrusive patches to SciPy? I haven't followed the discussion in scipy/scipy#15290 till the end but based on a quick look, there is a history of some having been upstreamed – in this case, patch 9/9 is a very small one and I would like to propose fixing it upstream after this PR here is complete/merged; it would (should) not break any existing changes and the syntax is probably more compatible across these different variants for Fortran compilers.

[rgommers]

is there a policy on upstreaming particular non-intrusive patches to SciPy

That all depends on whether the changes improve the SciPy code, or whether it's a workaround that's very specific to Pyodide. In this case, the original Fortran code seems correct and easier to read than the patched version - and this looks like an f2c issue, so I'd suggest not to upstream.

[hoodmane]

The problem seems to be that scipy/linalg/cython_lapack.cpython-312-wasm32-emscripten.so fails to load (which causes other libs to fail to load which eventually leads to the missing PyInit_ error). The loader error for cython_lapack is:

WebAssembly.instantiate(): Import #67 module="env" function="cbbcsd_": imported function does not match the expected type

which is frustratingly only the first of potentially many problems and also pretty darn vague. Checking the type of the symbol that it is getting (which presumably hasn't changed):

pyodide._module.resolveGlobalSymbol("cbbcsd_").sym.type()

we see that it takes 29 i32 params and returns one i32 result.

$ wasm-objdump linalg/cython_lapack.cpython-312-wasm32-emscripten.so -j Import -x | grep cbbcsd
 - func[67] sig=30 <env.cbbcsd_> <- env.cbbcsd_

shows that sig=30. Checking sig 30 shows:

$ wasm-objdump linalg/cython_lapack.cpython-312-wasm32-emscripten.so -j Type -x | grep 30
 - type[30] (i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32) -> nil

so the problem is the void return type. Of course.

[hoodmane]

Looks like _lapack_subroutines.h has 1416 different functions that are declared as returning void. Those will all need to be changed to int. The only place I find cbbcsd in the source is in cython_lapack_signatures.txt so that's probably the source of truth here. When this was going through fortran it was getting fixed by scipy_fix_cfile but now that it isn't we ideally should patch the sources.

[hoodmane]

Okay progress, now problem is chla_transtype_. It is defined with type (i32, i32, i32) -> nil but declared with type (i32) -> i32. This has a comment in f2c_fixes:

    chla_transtype is a special case. The OpenBLAS version of chla_transtype takes
    a return argument, whereas f2c thinks it should return the value.

Specifically:

line = line.replace("ret = chla_transtype(", "call chla_transtype(ret, 1,")

[hoodmane]

Where did all the build artifacts go? Meson seems to hide them better than the old build system did.

[agriyakhetarpal]

I am not sure what happens under cross-compilation, but the build artifacts typically show up inside a build-install directory in the source tree.

[hoodmane]

Hmm don't see any build-install directory. Could it be deleting them when its done or something?

[rgommers]

Where did all the build artifacts go? Meson seems to hide them better than the old build system did.

By default they go into a temporary directory if you use a plain python -m build (or pip), but you can add -Cbuild-dir=build and then all build artifacts are going under ./build/. More detailed logs are in build/meson-log/ and build artifacts are in corresponding subdirs of the source dirs (e.g., build/scipy/linalg/).

[hoodmane]

We should just add -Cbuild-dir=build always.

[agriyakhetarpal]

We should just add -Cbuild-dir=build always.

@hoodmane, where would you recommend I add this? I tried to follow along similar recipes but couldn't find any, and then I looked at how this is done for the Pyodide CLI in cli/build.py which seemed simple enough – pyodide build -C key=value. However, how do I add this for SciPy where CircleCI builds it out of its recipe (and add it just for SciPy, of course)?

[hoodmane]

Add it to backend_flags in the build section of the meta.yaml file.

[agriyakhetarpal]

Okay, it failed to build SciPy but this is fixable, based on https://mesonbuild.com/meson-python/how-to-guides/config-settings.html#how-to-guides-config-settings. Let me push a change to fix this and analyse which other modules fail to import after the changes to the cython_lapack_signatures.txt failed and chla_transtype_ is to be fixed.

[agriyakhetarpal]

I think I have tried all combinations in backend-flags: (I had previously tried backend_flags: but Pydantic reported an error because it could not validate that schema) under build: in scipy/meta.yaml:

• -Cbuild-dir=build
• --config-setting build-dir=build
• -C build-dir=build
• -Csetup-args="build-dir=build"

and none of them seems to work with meson-python since it reports an unknown option in all of them (the fourth one wouldn't have worked, of course). Am I doing something wrong here?

[rgommers]

The correct invocation should be:

 backend-flags: build-dir=build

Searching other build recipes in this repo for examples is usually helpful. The numpy package has a usage of backend-flags that I based the above off of.

[agriyakhetarpal]

Ah, there we go, thanks, @rgommers! I did do a search for this throughout the recipes before, and the reason I did not find anything was because I searched for backend_flags and not backend-flags, and I was looking for the -C option in specific (while I could have looked for build-dir as well, oops).

I feel this could be documented somewhere for use by others. There is a mention in the CHANGELOG, but it's not highlighted for package authors and maintainers on how to use backend-flags – maybe it's because not a lot of packages need it, especially those that use setuptools, because setuptools doesn't support config settings properly at the time of writing (and it doesn't document those well either where they work).

[hoodmane]

Documentation PRs are always very welcome. Our docs are very incomplete, and it would be nice for them to be less incomplete.