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

[ENH]: out-of-tree Pyodide builds in CI for Matplotlib #27870

Open
agriyakhetarpal opened this issue Mar 6, 2024 · 10 comments
Open

[ENH]: out-of-tree Pyodide builds in CI for Matplotlib #27870

agriyakhetarpal opened this issue Mar 6, 2024 · 10 comments

Comments

@agriyakhetarpal
Copy link

Problem

Hi there! I am opening this feature request to gauge ideas and comments about out-of-tree Pyodide builds, i.e., wasm32 wheels via the Emscripten toolchain for Matplotlib. In my most recent work assignment, I am working on improving the interoperability for the Scientific Python ecosystem of packages with Pyodide and with each other, which shall culminate with efforts towards bringing interactive documentation for these packages where they can then be run in JupyterLite notebooks, through nightly builds and wheels for these packages pushed to PyPI-like indices on Anaconda, at and during a later phase during the project.

This project is being tracked at Quansight-Labs/czi-scientific-python-mgmt#18.

Proposed solution

This issue proposes out-of-tree builds for matplotlib on its own CI and build infrastructure. I would be glad to work on this. This is how it would proceed, tentatively:

  1. A CI pipeline on GitHub Actions where Emscripten/Pyodide builds for the development version of matplotlib are pursued
  2. Testing the built wheels against a Pyodide wasm32 runtime virtual environment within the same workflow
  3. Fixing up and skipping failing tests as necessary based on current Pyodide limitations and ensuring that all relevant test cases pass.
@agriyakhetarpal
Copy link
Author

I already have a branch on my fork of Matplotlib where a few workflow runs have succeeded in the compilation and the wheel is being built: https://github.com/agriyakhetarpal/matplotlib/actions/runs/8161780336/job/22311265339.

I am working on running the tests, which are failing because of circular imports. It seems to be picking up the current (root) directory – I would appreciate any suggestions on how to run the test suite in this case (it works locally, but we are limited by the fact that the matplotlib.test() pathway has been deprecated now).

@agriyakhetarpal
Copy link
Author

agriyakhetarpal commented Mar 6, 2024

Based on https://matplotlib.org/devdocs/devel/testing.html#testing-released-versions-of-matplotlib, I am assuming I need to additionally check out the GitHub repository and copy the baseline images to where I installed the Matplotlib wheel...

Edit: done in https://github.com/agriyakhetarpal/matplotlib/actions/runs/8173325260/job/22345664858 where I am downloading the built wheel artifact in another job (that needs the first one to pass) and doing a sparse checkout of the baseline images to copy to the Matplotlib installation inside the virtual environment. I am facing issues with test collection – TypeError: the 'package' argument is required to perform a relative import for '.venv-pyodide.lib.python3.11.site-packages.matplotlib.tests' and I am unsure how to fix that!

@agriyakhetarpal
Copy link
Author

Another progress update via this workflow run – tests are now being collected, but fail to run because Matplotlib fails to be imported – this is due to the existence of the import threading statement1 in font_manager.py, figure.py, pyplot.py and some more files. I am not sure what alternatives https://github.com/pyodide/matplotlib-pyodide/ is using to get access to the plotting backends?

It is to note that the in-tree Pyodide build does contain patches to repeal the use of threads, which I can adopt:

  1. https://github.com/pyodide/pyodide/blob/main/packages/matplotlib/patches/0002-fix-threading.patch
  2. https://github.com/pyodide/pyodide/blob/main/packages/matplotlib/patches/fix-threading.patch

Footnotes

  1. https://github.com/pyodide/pyodide/issues/237

@agriyakhetarpal
Copy link
Author

Some more progress: I added a patch to disable the use of threading and skipped certain tests that were importing the threading standard module (threading is not supported in Pyodide yet). Next, I have split the build and test steps into two separate jobs that share artifacts (the wheel, in this case). The testing job then checks out the baseline images from the Matplotlib repository and copies them into the installed Matplotlib inside the virtual environment set up by Pyodide, and runs the pytest invocation from there (this is due to the lack of a matplotlib.test() API, which has been deprecated).

It seems that my patch is not entirely correct, because the tests seem to proceed to collect, but fail and the interpreter prints out a random seed (supposedly set by NumPy) without any other stack trace. This makes it difficult to debug what is happening.

Here are the logs: https://github.com/agriyakhetarpal/matplotlib/actions/runs/8187758884/job/22390718867#step:11:43

@tacaswell
Copy link
Member

We used to have the threading imports protected, but removed them in #23073 because we thought pyodide no loger needed them.

Happy to take those back in if needed.

@agriyakhetarpal
Copy link
Author

agriyakhetarpal commented Mar 8, 2024

Hi @tacaswell, I did have further progress after my last comment and disabled the threading imports where needed – but it looks like this could be, and remain, stalled for a while due to some unresolved symbols (please see the linked issue above) where my inexperience beats me.

I do have working patches and a reliable CI job in a branch on my fork where the rest of the build procedure seems to be working, though. It isn't ready for a PR yet until I have a fully working test suite that completes itself, but I'm more than happy to open a draft PR if by doing so it can meet more eyes (and possibly drive progress towards resolution).

@rgommers
Copy link

I'd switch to the testing method that scikit-learn uses to see if that fixes the missing symbol issues. If that doesn't help, let's look at them in more detail.

@QuLogic
Copy link
Member

QuLogic commented Jun 5, 2024

@thomasjpfan and I were looking at this at the Dev Summit yesterday. Unfortunately, I forgot that this issue existed, and somewhat repeated what you've already done, but fortunately haven't done much that would replace it. I do think we can take on a couple of the patches more generally instead of leaving them downstream.

One thing that has made this a bit easier is that cibuildwheel just merged support for building wasm wheels. There's been some hacks, but I'm able to reach a point that I can build locally and import, but plotting fails with a crash in FreeType trampolining. Here is the backtrace of that event:

+ python 
Python 3.12.1 (main, May 27 2024, 13:56:13) [Clang 19.0.0git (https:/github.com/llvm/llvm-project 0a8cd1ed1f4f35905df318015b on emscripten
Type "help", "copyright", "credits" or "license" for more information.
>>> import matplotlib.pyplot as plt
>>> fig, ax = plt.subplots()
>>> fig.draw_without_rendering()
Stack (most recent call first):
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/backends/backend_agg.py", line 220 in get_text_width_height_descent
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/text.py", line 77 in _get_text_metrics_with_cache_impl
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/text.py", line 69 in _get_text_metrics_with_cache
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/text.py", line 373 in _get_layout
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/text.py", line 956 in get_window_extent
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/axis.py", line 1350 in _get_ticklabel_bboxes
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/axis.py", line 1423 in draw
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/artist.py", line 72 in draw_wrapper
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/image.py", line 132 in _draw_list_compositing_images
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/axes/_base.py", line 3113 in draw
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/artist.py", line 72 in draw_wrapper
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/image.py", line 132 in _draw_list_compositing_images
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/figure.py", line 3155 in draw
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/artist.py", line 72 in draw_wrapper
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/artist.py", line 95 in draw_wrapper
  File "/tmp/cibw-run-vhgunnc9/cp312-pyodide_wasm32/venv-test/lib64/python3.12/site-packages/matplotlib/figure.py", line 3171 in draw_without_rendering
  File "<stdin>", line 1 in <module>
RuntimeError: null function or function signature mismatch
    at wasm://wasm/00213ff6:wasm-function[350]:0x1ef03
    at wasm://wasm/00213ff6:wasm-function[201]:0xf3ab
    at wasm://wasm/00213ff6:wasm-function[119]:0x7e6a
    at wasm://wasm/00213ff6:wasm-function[149]:0xb05e
    at _PyEM_TrampolineCall_JS (/home/elliott/.cache/cibuildwheel/.pyodide-xbuildenv-0.26.0/0.26.0/xbuildenv/pyodide-root/dist/pyodide.asm.js:10:125866)
    at wasm://wasm/0267d6e2:wasm-function[1317]:0x1ce387
    at wasm://wasm/0267d6e2:wasm-function[1143]:0x1c35da
    at wasm://wasm/0267d6e2:wasm-function[3408]:0x2a05c2
    at wasm://wasm/0267d6e2:wasm-function[3409]:0x2a4e5c
    at wasm://wasm/0267d6e2:wasm-function[1148]:0x1c3971

"matplotlib/backends/backend_agg.py", line 220 in get_text_width_height_descent is a call to an extension-level function, but unfortunately, I don't see much detail in the WASM-side traceback at the end there. @thomasjpfan was able to figure out a bit more info when in the browser that pointed to a call to FT_Load_Glyph, but that's where we're stuck as there doesn't seem to be any reason for that to mismatch.

If you happen to know how to get wasm to compile with a bit more debug information, that could maybe help me move forward a bit.

@agriyakhetarpal
Copy link
Author

Thank you, @QuLogic and @thomasjpfan! I haven't looked at this in a while but this remains very much on my radar. @ryanking13 had previously shared https://pyodide.org/en/stable/development/debugging.html with me which has some information on function signature mismatches, where compiling with -g2 instead of should reveal some more information. I'll be happy to start trying this again and dive deeper.

@QuLogic
Copy link
Member

QuLogic commented Jun 6, 2024

Thanks, I've looked through that page and managed to find some info about the wasm debug tools that was helpful. We need to disable some of our build options in order to get more symbols (more than just passing -g2), but I think I've been successful. Unfortunately, the native runner doesn't seem to print symbols, but this may help in-browser though I've not had a chance to try that out. I'll try and post what I have cobbled together in a little while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants