pyodide patches

[ev-br]

To build scipy on WASM, pyodide carries several patches. Some of them might make sense to just include to scipy, and this issue is to track this. There are fourteen patches in total at https://github.com/hoodmane/pyodide/tree/main/packages/scipy/patches. Here's a rough grouping with an initial shot at a triage. The grouping and proposed decisions are very much up to discussion!

Low-hanging fruits, SciPy-specific, look straightforward to pick up: EDIT: the decision in gh-17414 is to not merge these three:

• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0004-int-to-string.patch Fixes an implicit int-to-character conversion. The whole code looks very much Fortran-66. Yikes.
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0009-Rename-_page_trend_test.py-to-prevent-test-unvendori.patch This is AFAICS specific to pyodide process, but looks harmless for us.
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0013-Add-extra-END-to-prini.f.patch something weird in id_decom in scipy/linalg. Looks harmless. If it fixes it for pyodide, it does I guess. EDIT: Was considered and rejected in MAINT Add an extra END to prini.f #15954.
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0014-BUG-Fix-signature-of-D_IIR_forback-1-2.patch
  Has been applied already in BUG Fix signature of D_IIR_forback(1,2) #15955

Maybe hold on

• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0001-Fix-dstevr-in-special-lapack_defs.h.patch
  These lines were last touched in 0ad7e3a3f55 which added ILP64 support to distutils-based builds. Hold on this patch until ILP64 is supported in regular meson builds.

Apply to scipy and submit to SuperLU upstream
SuperLU appears to be active, https://github.com/xiaoyeli/superlu, so they might be interested in these three patches

• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0007-gemm_-no-const.patch
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0008-make-int-return-values.patch
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0010-sasum-returns-double-not-float.patch This patch seems incorrect, cf pyodide patches #17413 (comment)

A right course of action seems to be to submit them upstream, and apply to SciPy if accepted.

Distutils-specific

I think these can be ignored as distutils is on its way out:

• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0005-disable-blas-detection.patch
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0011-skip-fortran-fails-to-link.patch
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0012-Disable-lapack-detection.patch
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0003-Add-lapack_extras-to-linalg-setup.py.patch

pyodide specific

• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0002-loadDynamicLibrary-flapack.patch
• https://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0006-fix-fotran-files-minpack.patch

The second one we certainly do not want to apply: it undoes the recursive subroutine definitions. If pyodide needs it, it will need to also undo #15997 and #16053 which came in for scipy 1.9.

Am going to submit a PR for low-hanging fruits already.

[hoodmane]

Thanks for looking into this @ev-br!

• Patch 1 fix dstevrhttps://github.com/hoodmane/pyodide/blob/main/packages/scipy/patches/0001-Fix-dstevr-in-special-lapack_defs.h.patch may be due to an ABI difference between CLAPACK and LAPACK (C strings vs fortran strings). These ABI differences cause us a lot of trouble actually. Probably not appropriate to upstream. Though there is some vague possibilities that there is in fact no real ABI difference and what is happening is that scipy is calling a bunch of functions that take constant length strings with extra length arguments that they ignore. I think it's specific to us because we use CLAPACK and not appropriate for upstreaming, but I'm not 100% on this.
• I think patch 10 sasum-returns-double-not-float is an actual platform independent bug which ought to be upstreamed.
• Patch 8 is harmless but of dubious value for regular workflows.
• I'm not sure about patch 7 but when we compile with flags that make the compiler const-strict it gets mad. This may be special to our weird build pipeline though.

[hoodmane]

Also, taking another look at 0004-int-to-string.patch I think it might actually be reasonable to upstream? I was mostly voting against upstreaming the other two patches but I didn't check that one out carefully enough. Maybe @WarrenWeckesser could look into this one too and tell us what he thinks?

The prini.f patch we dropped from Pyodide after @WarrenWeckesser told me it was wrong.

[ev-br]

I think patch 10 sasum-returns-double-not-float is an actual platform independent bug which ought to be upstreamed.

Looking at it, sasum is a BLAS level 1 function, where the first letter indicates single precision. The netlib source concurs: https://netlib.org/lapack/explore-html/df/d28/group__single__blas__level1_gafc5e1e8d9f26907c0a7cf878107f08cf.html

real function sasum
...
real stemp
...
sasum = stemp
RETURN

MKL docs agree: https://www.intel.com/content/www/us/en/develop/documentation/onemkl-developer-reference-fortran/top/blas-and-sparse-blas-routines/blas-routines/blas-level-1-routines-and-functions/asum.html

Output Parameters
res
REAL for sasum

f2py wrapper is scipy also concurs:
the definition of asum is https://github.com/scipy/scipy/blob/main/scipy/linalg/fblas_l1.pyf.src#L420 and <ftypereal3> is ! <ftypereal3=real,real> https://github.com/scipy/scipy/blob/main/scipy/linalg/fblas_l1.pyf.src#L31

cython_blas.pyx generators AFAICS also generate float sasum(...)

So I wonder if it's also not a build system glitch that you need this patch?

[hoodmane]

Yeah... It's a little hard for me to fathom just what sort of build system glitch would change the return type of the function like that but it would be a whole lot weirder for the whole rest of the world to be wrong.

[ilayn]

Do we still have anything actionable here?

[agriyakhetarpal]

We currently have eighteen patches in total, most already discussed above. Four new patches have been added. One of the patches concerns the build system machinery where I had to drop all of the numpy.f2py generators: https://github.com/pyodide/pyodide/blob@7176d19/packages/scipy/patches/0015-Remove-f2py-generators.patch, which would be good to resolve/drop as it also happens to conflict with the contents of other patches that are not upstreamable, and doesn't actually cater to any implementation code. My memory is a little murky so I can't recall why we needed it, but I remember that @ryanking13 was working on a potential fix on the Pyodide build tooling side, which has been stalled for some time: pyodide/pyodide-build#21

I upstreamed https://github.com/pyodide/pyodide/blob@7176d19/packages/scipy/patches/0011-Remove-fpchec-inline-if-then-endif-constructs.patch already via gh-21365. It can be dropped with the SciPy 1.15 in-tree update in Pyodide (see below) :)

Another patch: https://github.com/pyodide/pyodide/blob@7176d19/packages/scipy/patches/0016-Make-sf_error_state_lib-a-static-library.patch was applied because as per @hoodmane, Meson has been incorrectly assuming that wasm.ld does not support shared libraries: https://github.com/mesonbuild/meson/blob/acb32b2efdd0aae677c5b2622c0175011f65fc6c/mesonbuild/linkers/linkers.py#L1020

Lastly, https://github.com/pyodide/pyodide/blob@7176d19/packages/scipy/patches/0018-Fix-lapack-larfg-function-signature.patch was added to partially revert some old code in gh-8300 that was silencing some compiler warnings. Hence, if we can fix it here, this looks like a reasonable candidate to drop.

Besides these, we're hoping to use the new Flang from LLVM 19 (and now 20) – we'll have to use the one from conda-forge, as it contains patches to enable the WASM target. The tracking issue for it is here: pyodide/pyodide#5268. I also hope to come back to pyodide/pyodide#5297, which was stalled due to the update that happened in gh-21110 which f2c can't interpret, but TBH, I haven't worked on it much for some time now and it could probably be that there's not much to do besides this to get SciPy to build.

[ilayn]

I'll take that as a yes 😅

Hence, if we can fix it here, this looks like a reasonable candidate to drop.

Let me fix soon, that so it goes into 1.16

[agriyakhetarpal]

With SciPy 1.16, I've ported the ARPACK C rewrite via gh-22748 – I'm working on it at pyodide/pyodide-recipes#141. The PR includes more details and additional context. Thank you for your great work on this front, @ilayn! It's truly appreciated!

[ilayn]

Hi @agriyakhetarpal apologies for the delayed response. I'm making the structure slightly better for a C project but in the meantime we renamed our version of ARPACK to avoid confusion and API changes here is how it looks #23274

[agriyakhetarpal]

Hi @ilayn, no worries about the delay! The C-rewritten ARPACK compiles pretty well already, with the caveat of us having to change the voids to ints of course – which is more of a problem stemming from the Fortran compiler infrastructure that Pyodide uses (xref gh-15290) rather than one in SciPy. The restructure you're doing in gh-23274 looks great (I'm not an expert at it). Therefore, having all the signatures consolidated in scipy/sparse/linalg/_eigen/arpack/arnaud/src/blaslapack_declarations.h is always helpful. :)