Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

WASM build#262

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
agriyakhetarpal wants to merge109 commits intoflintlib:main
base:main
Choose a base branch
Loading
fromagriyakhetarpal:bld/wasm
Draft

Conversation

@agriyakhetarpal
Copy link
Contributor

@agriyakhetarpalagriyakhetarpal commentedFeb 27, 2025•
edited
Loading

Closes#234

  • Build
    • libgmp
    • libmpfr
    • flint
    • python-flint
  • Test

@agriyakhetarpal
Copy link
ContributorAuthor

I've been testing it a bit through a PR in my fork, and it all builds well, up to one point in the Cython sources where it fails in step 107/114:https://github.com/agriyakhetarpal/python-flint/actions/runs/13576857716/job/37954995494?pr=1

with the following error trace:

FAILED: src/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/meson-generated_src_flint_types__gr.pyx.c.o /tmp/tmp8t79rr74/cc -Isrc/flint/types/_gr.cpython-312-wasm32-emscripten.so.p -Isrc/flint/types -I../src/flint/types -I/opt/hostedtoolcache/Python/3.12.9/x64/include/python3.12 -I/home/runner/work/python-flint/python-flint/wasm-library-dir/include -fvisibility=hidden -fdiagnostics-color=always -DNDEBUG -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -O3 -fPIC -MD -MQ src/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/meson-generated_src_flint_types__gr.pyx.c.o -MF src/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/meson-generated_src_flint_types__gr.pyx.c.o.d -o src/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/meson-generated_src_flint_types__gr.pyx.c.o -c src/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/src/flint/types/_gr.pyx.csrc/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/src/flint/types/_gr.pyx.c:19051:65: error: incompatible integer to pointer conversion passing 'mp_limb_t' (aka 'unsigned long') to parameter of type 'const fmpz *' (aka 'const long *') [-Wint-conversion] 19051 |   gr_ctx_init_fq_nmod(__pyx_v_ctx->__pyx_base.__pyx_base.ctx_t, __pyx_v_p, __pyx_v_d, __pyx_v_name);       |                                                                 ^~~~~~~~~/home/runner/work/python-flint/python-flint/wasm-library-dir/include/flint/gr.h:1366:53: note: passing argument to parameter 'p' here 1366 | void gr_ctx_init_fq_nmod(gr_ctx_t ctx, const fmpz_t p, slong d, const char * var);      |                                                     ^src/flint/types/_gr.cpython-312-wasm32-emscripten.so.p/src/flint/types/_gr.pyx.c:19150:65: error: incompatible integer to pointer conversion passing 'mp_limb_t' (aka 'unsigned long') to parameter of type 'const fmpz *' (aka 'const long *') [-Wint-conversion] 19150 |   gr_ctx_init_fq_zech(__pyx_v_ctx->__pyx_base.__pyx_base.ctx_t, __pyx_v_p, __pyx_v_d, __pyx_v_name);       |                                                                 ^~~~~~~~~/home/runner/work/python-flint/python-flint/wasm-library-dir/include/flint/gr.h:1367:53: note: passing argument to parameter 'p' here 1367 | void gr_ctx_init_fq_zech(gr_ctx_t ctx, const fmpz_t p, slong d, const char * var);      |                                                     ^2 errors generated.

@agriyakhetarpal
Copy link
ContributorAuthor

FWIW; I hardly have any Cython development experience, so I will have to wrap my head around this a bit, unless someone has an idea for a fix. The basis for the failure here is that WASM is stricter about type conversions than compilation for other platforms, which is why this mismatch is causing errors.

@agriyakhetarpal
Copy link
ContributorAuthor

Okay – the cause of the error, more specifically, is thatgr_fq_nmod_ctx andgr_fq_zech_ctx pass aulong parameterp

@cython.no_gc
cdefclass gr_fq_zech_ctx(gr_scalar_ctx):
cdef ulong p
cdef slong d
@staticmethod
cdef inline gr_fq_zech_ctx _new(ulong p, slong d,char* name):
cdef gr_fq_zech_ctx ctx
ctx= gr_fq_zech_ctx.__new__(gr_fq_zech_ctx)
ctx.p= p
ctx.d= d
gr_ctx_init_fq_zech(ctx.ctx_t, p, d, name)
ctx._init=True
return ctx

butflint/gr.h hasfmpz_t in version 3.0.1:https://github.com/flintlib/flint/blob/c05007be863b1aae7221f11a9c135d673d968638/src/gr.h#L1366-L1367

and this has apparently been fixed in version v3.1.0. I'll try updatingflint to v3.1.0 (before proceeding to try out v3.1.2, if I do)

@agriyakhetarpal
Copy link
ContributorAuthor

Successful build:https://github.com/agriyakhetarpal/python-flint/actions/runs/13577635441?pr=1 ✨

The next task is to run tests, which I can take a look at later in the day.

@oscarbenjamin
Copy link
Collaborator

I haven't looked through this yet but thanks!

@oscarbenjamin
Copy link
Collaborator

butflint/gr.h hasfmpz_t in version 3.0.1:

Best to keep with latest FLINT version for the pyodide build. There is no need to support a range of versions in pyodide. Generally python-flint tracks the latest version but has some#ifdef type stuff for older versions so that someone could build against them. No one is going to build against old versions in WASM though.

@agriyakhetarpal
Copy link
ContributorAuthor

Sounds good to me! I think the tests will have a bunch of function signature mismatches, though 😬 based on what I noticed through my fork's run. Also, building itself takes ~20 minutes in total – we could get to half of that if we have prebuilt WASM binaries. I guess they could be hosted somewhere in a GitHub release when things are ready.

@agriyakhetarpal
Copy link
ContributorAuthor

This can take a bit of time. I'm disabling tests and running it in on my fork to see how widespread the signature mismatches are, and I managed to gettest_all.py to run till the end – now we have some more intest_docstrings.py. My next tasks will be to debug these mismatches, which I haven't had fun doing, as, i. compilation of these libraries to WASM is messy on a macOS and ii. a test here contains several assert statements, instead of several tests containing a few of them. Please bear with me as I proceed with this slowly and steadily. We could also wholly ignoretest_docstrings.py if you're not too keen on that being tested. (I think it's good to test, though).

@oscarbenjamin
Copy link
Collaborator

I don't quite understand what the problem with the tests is. Is it just caused by the Cython/C signatures not matching?

There is now FLINT 3.2.0-rc1. We could perhaps update all the Cython declarations to that version and use that for the WASM build. I'm not sure when 3.2.0 final will be released but we would want to update to that straight away when it is.

@agriyakhetarpal
Copy link
ContributorAuthor

agriyakhetarpal commentedMar 3, 2025•
edited
Loading

I don't quite understand what the problem with the tests is. Is it just caused by the Cython/C signatures not matching?

Yes, majorly. If there's even a slight mismatch between the upstream signatures for Flint and the ones we have here, WASM's type safety guarantees that it will terminate the program, which makes Pyodide raise a fatal error. These can happen at the time of linking (like when it happened with version 3.0.1 above), or at runtime, like we are facing now. I have to drop these completely instead of xfailing them as the running Pyodide interpreter is no longer possible to be used after it has encountered fatal errors.

There are so many of these with SciPy, especially from its wrapping of Fortran libraries that we can't compile to WASM directly without having tof2c them over. I hope the new Flang from LLVM-19 can do this better:pyodide/pyodide#5268

There is now FLINT 3.2.0-rc1. We could perhaps update all the Cython declarations to that version and use that for the WASM build. I'm not sure when 3.2.0 final will be released but we would want to update to that straight away when it is.

Yes, this sounds good to me, and I hope they've fixed a few of these upstream. I could start another PR for this when I have enough time to do so; I assume it will help me learn Cython a bit. :)

@oscarbenjamin
Copy link
Collaborator

I've openedgh-264 to bump the FLINT version to 3.2.0-rc1 and update all of the Cython declarations.

The Cython declarations are set from the FLINT docs by running thebin/all_rst_to_pxd.py script. There can often be mismatches resulting from parsing the signatures from the docs rather than the actual C code e.g. the docs may be inaccurate but usually in the Cython -> C -> compiled extension modules the differences even out largely because a lot of FLINT's functions are not actually called directly by python-flint.

agriyakhetarpal reacted with rocket emoji

@oscarbenjamin
Copy link
Collaborator

I've openedgh-264 to bump the FLINT version to 3.2.0-rc1 and update all of the Cython declarations.

I've just merged this to main. If you rebase/merge with main then this PR will get the updated declarations.

There might still be some wrong declarations that we can fix manually for now but then upstream to FLINT afterwards.

@oscarbenjamin
Copy link
Collaborator

More deletions are needed. This time I checked it compiles and runs:

diff --git a/src/mag/test/main.c b/src/mag/test/main.cindex 4549b0155..0789414fc 100644--- a/src/mag/test/main.c+++ b/src/mag/test/main.c@@ -24,8 +24,6 @@ #include "t-cosh.c" #include "t-div.c" #include "t-div_lower.c"-#include "t-d_log_lower_bound.c"-#include "t-d_log_upper_bound.c" #include "t-dump_file.c" #include "t-dump_str.c" #include "t-exp.c"@@ -54,8 +52,6 @@ #include "t-root.c" #include "t-rsqrt.c" #include "t-rsqrt_lower.c"-#include "t-set_d_2exp_fmpz.c"-#include "t-set_d.c" #include "t-set_ui.c" #include "t-set_ui_lower.c" #include "t-sinh.c"@@ -79,8 +75,6 @@ test_struct tests[] =     TEST_FUNCTION(mag_cosh),     TEST_FUNCTION(mag_div),     TEST_FUNCTION(mag_div_lower),-    TEST_FUNCTION(mag_d_log_lower_bound),-    TEST_FUNCTION(mag_d_log_upper_bound),     TEST_FUNCTION(mag_dump_file),     TEST_FUNCTION(mag_dump_str),     TEST_FUNCTION(mag_exp),@@ -109,8 +103,6 @@ test_struct tests[] =     TEST_FUNCTION(mag_root),     TEST_FUNCTION(mag_rsqrt),     TEST_FUNCTION(mag_rsqrt_lower),-    TEST_FUNCTION(mag_set_d_2exp_fmpz),-    TEST_FUNCTION(mag_set_d),     TEST_FUNCTION(mag_set_ui),     TEST_FUNCTION(mag_set_ui_lower),     TEST_FUNCTION(mag_sinh),

@agriyakhetarpal
Copy link
ContributorAuthor

agriyakhetarpal commentedMay 26, 2025•
edited
Loading

Ah, yes, this time it compiles. Here is the output fromemmake make check:

make: make checkadd_ssaaaa...add_ssaaaa                                        0.00   (PASS)add_sssaaaaaa...add_sssaaaaaa                                     0.09   (PASS)flint_clz...flint_clz                                         0.00   (PASS)flint_ctz...flint_ctz                                         0.00   (PASS)flint_fprintf...flint_fprintf                                     0.00   (FAIL)Could not open temporary file"tmp"Aborted()/Users/agriyakhetarpal/Desktop/flint/build/test/main:380  var e = new WebAssembly.RuntimeError(what);          ^RuntimeError:Aborted(). Build with -sASSERTIONSfor more info.    at abort (/Users/agriyakhetarpal/Desktop/flint/build/test/main:380:11)    at __abort_js (/Users/agriyakhetarpal/Desktop/flint/build/test/main:3388:7)    at main.wasm.abort (wasm://wasm/main.wasm-083fa0d6:wasm-function[8641]:0x4647b4)    at main.wasm.flint_abort (wasm://wasm/main.wasm-083fa0d6:wasm-function[40]:0x90a0)    at main.wasm.main (wasm://wasm/main.wasm-083fa0d6:wasm-function[38]:0x9010)    at Module._main (/Users/agriyakhetarpal/Desktop/flint/build/test/main:3728:102)    at callMain (/Users/agriyakhetarpal/Desktop/flint/build/test/main:3754:15)    at doRun (/Users/agriyakhetarpal/Desktop/flint/build/test/main:3793:24)    at run (/Users/agriyakhetarpal/Desktop/flint/build/test/main:3806:5)    at removeRunDependency (/Users/agriyakhetarpal/Desktop/flint/build/test/main:348:7)Node.js v20.18.0make:*** [build/test/main_TEST_RUN] Error 1emmake: error:'make check' failed (returned 2)

The failing test is fine; we can't access the file system without mounting aNodeFS or a similar one. I don't think we should dwell on it.

Note that I had to manually modify the generatedMakefile as it did not interpret thatbuild/test/main was a JavaScript file, so I had to prependnode before the%_TEST_RUN: % rules. It should buildmain.js if theHOST_OS matchesemscripten and use Node.js in that case by default – but that was just some misconfiguration that we can deal with. At least from this, we can see that most tests pass and WASM compilation works.

I hope we are not missing any tests, though. Looks like we are :/

@oscarbenjamin
Copy link
Collaborator

I hope we are not missing any tests, though. Looks like we are :/

Yes, usually there are more than 5 tests!

agriyakhetarpal reacted with eyes emoji

@agriyakhetarpal
Copy link
ContributorAuthor

agriyakhetarpal commentedMay 27, 2025•
edited
Loading

I addedNODERAWFS=1 to myTESTCFLAGS for allowing access to my file system, and this has let the tests go forward much more:

make: make checkadd_ssaaaa...add_ssaaaa                                        0.00   (PASS)add_sssaaaaaa...add_sssaaaaaa                                     0.04   (PASS)flint_clz...flint_clz                                         0.00   (PASS)flint_ctz...flint_ctz                                         0.00   (PASS)flint_fprintf...flint_fprintf                                     0.01   (PASS)flint_printf...flint_printf                                          (SKIPPED)memory_manager...memory_manager                                    0.00   (PASS)sdiv_qrnnd...sdiv_qrnnd                                        0.01   (PASS)smul_ppmm...smul_ppmm                                         0.01   (PASS)flint_sort...flint_sort                                        0.00   (PASS)sub_dddmmmsss...sub_dddmmmsss                                     0.05   (PASS)sub_ddmmss...sub_ddmmss                                        0.00   (PASS)udiv_qrnnd...udiv_qrnnd                                        0.01   (PASS)udiv_qrnnd_preinv...udiv_qrnnd_preinv                                 0.00   (PASS)umul_ppmm...umul_ppmm                                         0.01   (PASS)thread_pool...thread_pool                                       0.10   (PASS)thread_support_parallel_binary_splitting...thread_support_parallel_binary_splitting          0.00   (PASS)thread_support_parallel_do...thread_support_parallel_do                        0.00   (PASS)n_addmod...n_addmod                                          0.00   (PASS)n_cbrt_binary_search...n_cbrt_binary_search                              0.00   (PASS)n_cbrt...n_cbrt                                            0.01   (PASS)n_cbrt_chebyshev_approx...n_cbrt_chebyshev_approx                           0.00   (PASS)n_cbrtrem...n_cbrtrem                                         0.01   (PASS)n_clog_2exp...n_clog_2exp                                       0.00   (PASS)n_clog...n_clog                                            0.00   (PASS)compute_primes...Aborted(OOM)/Users/agriyakhetarpal/Desktop/flint/build/ulong_extras/test/main:380  var e = new WebAssembly.RuntimeError(what);          ^RuntimeError: Aborted(OOM). Build with -sASSERTIONSfor more info.    at abort (/Users/agriyakhetarpal/Desktop/flint/build/ulong_extras/test/main:380:11)    at abortOnCannotGrowMemory (/Users/agriyakhetarpal/Desktop/flint/build/ulong_extras/test/main:3949:7)    at _emscripten_resize_heap (/Users/agriyakhetarpal/Desktop/flint/build/ulong_extras/test/main:3955:7)    at main.wasm.sbrk (wasm://wasm/main.wasm-084fa6f2:wasm-function[8876]:0x4812bd)    at main.wasm.emscripten_builtin_malloc (wasm://wasm/main.wasm-084fa6f2:wasm-function[8868]:0x47f4c6)    at main.wasm._flint_malloc (wasm://wasm/main.wasm-084fa6f2:wasm-function[155]:0x186f6)    at main.wasm.flint_malloc (wasm://wasm/main.wasm-084fa6f2:wasm-function[147]:0x1850f)    at main.wasm.n_compute_primes (wasm://wasm/main.wasm-084fa6f2:wasm-function[189]:0x1a0b1)    at main.wasm.n_primes_arr_readonly (wasm://wasm/main.wasm-084fa6f2:wasm-function[271]:0x1fe54)    at main.wasm.test_compute_primes (wasm://wasm/main.wasm-084fa6f2:wasm-function[27]:0x50f5)Node.js v20.18.0make:*** [build/ulong_extras/test/main_TEST_RUN] Error 1emmake: error:'make check' failed (returned 2)

The relevant section of themain file that Emscripten has set up looks like this (lines 3944–3956), but I don't think this helps much:

vargetHeapMax=()=>HEAPU8.length;var_emscripten_get_heap_max=()=>getHeapMax();varabortOnCannotGrowMemory=(requestedSize)=>{abort('OOM');};var_emscripten_resize_heap=(requestedSize)=>{varoldSize=HEAPU8.length;// With CAN_ADDRESS_2GB or MEMORY64, pointers are already unsigned.requestedSize>>>=0;abortOnCannotGrowMemory(requestedSize);};

And from the CI jobs for FLINT, I see that none of thenmod_vec_,flint_mpn,fmpz,fmpq,arb,gr_ tests ran; this isn't the full test suite either, so it would be nice if there were apytest-like way to skip tests that we know are going to abort the program without having to recompile everything, or to allow a selective portion of the tests of our choice to run, similar to the-k option.

@oscarbenjamin
Copy link
Collaborator

it would be nice if there were apytest-like way to skip tests that we know are going to abort the program without having to recompile everything, or to allow a selective portion of the tests of our choice to run, similar to the-k option.

I'm not sure what options there are but you can just run the individual test files like:

$ build/acb/test/mainacb_acos...acb_acos                                          0.00   (PASS)acb_acosh...acb_acosh                                         0.00   (PASS)acb_agm1...acb_agm1                                          0.15   (PASS)acb_agm...acb_agm                                           0.09   (PASS)...

You can list the hundred or so test executables and maybe put them in a script and comment out the ones you don't want:

$ls build/*/test/mainbuild/acb_calc/test/main               build/fq_default_mat/test/mainbuild/acb_dft/test/main                build/fq_default_poly_factor/test/mainbuild/acb_dirichlet/test/main          build/fq_default_poly/test/mainbuild/acb_elliptic/test/main           build/fq_default/test/main...

Given the things that currently don't work in this PR I would be most interested inacb_modular,nf,nf_elem, and thegr_* tests.

@oscarbenjamin
Copy link
Collaborator

Okay, so the CI job passes now. I think that a good milestone here would be to get something merged so that we have emscripten running in CI and nightly wheels being uploaded even if there are some outstanding known issues.

I suggest we:

  • Preserve this PR with its many commits and long discussion intact for posterity.
  • Open a new PR that squashes the commits and does any remaining clean ups.
  • Then I will follow up after with something to clean up the test changes and come up with a better solution for the doctests that are skipped or sorting of polynomial factors.

Lastly what remains is to see if we can identify and fix the problems withacb_modular_hilbert_class_poly andgr_nf but unless we can quickly identify the issue there I think it is better to leave that for later.

@agriyakhetarpal
Copy link
ContributorAuthor

Sounds good to me! I will create another PR from here and also obtain some additional WASM test results from FLINT-main in a new issue – hopefully by tonight or early tomorrow. As most of the tests pass, I think the next steps, after yours, are to:

  • get patches for updated versions of FLINT and python-flint accordingly tohttps://github.com/pyodide/pyodide-recipes/ so that we can ensure that they are included in Pyodide 0.28 (which will come in a few weeks from now).
  • use Pyodide 0.28 for testing SymPy in its Pyodide CI, and include the python-flint tests that were being skipped because it's not installed, and address any problems there (perhaps it makes sense to address problems here first, I can't say)
  • Install python-flint alongside SymPy for SymPy Live, and try to resolveDisplay the SymPy version in the shell prompt sympy/live#23

@fredrik-johansson
Copy link
Collaborator

Not sure how urgent this is: should we hold up the 3.3 release of FLINT to try to get all tests to pass on WASM first, or is it OK if we release 3.3 now and push any further fixes for WASM compatibility to a future release?

@oscarbenjamin
Copy link
Collaborator

I think for now we can track FLINT's main branch for the WASM build so I think the FLINT release doesn't matter. At least the current state in this PR worked good enough with FLINT main and so if FLINT 3.3 is released now then it should be possible to get at least as good as we have here by pinning to FLINT 3.3.

How long it would take to iron these things out is unknown so it is probably best not to wait on it.

fredrik-johansson and agriyakhetarpal reacted with thumbs up emoji

@oscarbenjamin
Copy link
Collaborator

I'm reopening to test aftergh-286 which updates the FLINT version to 3.3.0 and also updates the cython declarations for FLINT C functions.

@oscarbenjamin
Copy link
Collaborator

oscarbenjamin commentedJun 11, 2025•
edited
Loading

The problem withfmpz_poly.hilbert_class_poly seems resolved. The problem with gr_nf is not (and is the only outstanding issue in this PR):

../.venv-pyodide/lib/python3.13/site-packages/flint/test/test_docstrings.py::test_docstrings[flint.types._gr.new13] PASSEDPyodide has suffered a fatal error. Please report this to the Pyodide maintainers.The cause of the fatal error was:RuntimeError: memory access out of bounds    at wasm://wasm/020eeec2:wasm-function[9770]:0x4883e2    at wasm://wasm/02dce452:wasm-function[15599]:0x9273aa    at wasm://wasm/02dce452:wasm-function[15583]:0x926e37    at wasm://wasm/02dce452:wasm-function[14736]:0x8d16a2    at wasm://wasm/02fbdc16:wasm-function[422]:0xc40a5    at wasm://wasm/020eeec2:wasm-function[[230](https://github.com/flintlib/python-flint/actions/runs/15597884038/job/43932170844#step:15:231)6]:0x1b8b1d    at wasm://wasm/02fbdc16:wasm-function[685]:0xf20ad    at wasm://wasm/020eeec2:wasm-function[2306]:0x1b8b1d    at wasm://wasm/020eeec2:wasm-function[1962]:0x1a0d0a    at wasm://wasm/020eeec2:wasm-function[1949]:0x1a01c4 {  pyodide_fatal_error: true}

https://github.com/flintlib/python-flint/actions/runs/15597884038/job/43932170844

Thegr_nf issue was revealed again by reenabling the doctests inee057b4.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@oscarbenjaminoscarbenjaminoscarbenjamin left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Nightly wheels and WASM/pyodide builds

4 participants

@agriyakhetarpal@oscarbenjamin@albinahlback@fredrik-johansson
[8]ページ先頭
©2009-2025 Movatter.jp