matplotlib-pyodide icon indicating copy to clipboard operation
matplotlib-pyodide copied to clipboard

matplotlib: savefig() produces TypeError: Invalid argument type in ToBigInt operation

Open ashley-hh opened this issue 4 years ago • 11 comments

Hello fellow pyodide users and mods!! Thank you for your hard work, this is an awesome project and I've been using it heavily!! Most recently, I've been trying to plot clustermaps from seaborn. This works amazingly on Chrome and somewhat well on Firefox, but not at all on Safari. This is the error that I've been getting when I call savefig() from the matplotlib library:

Screen Shot 2021-11-02 at 2 22 07 AM

Chasing down the most recent stack call, it's this function: Screen Shot 2021-11-02 at 2 23 26 AM

Before I go even further down the rabbit hole to bisect this error, has anyone already encountered this and know how to get around it? Is this even the right place to ask this question? This is my first experience with cross-platform software, so any advice would be greatly appreciated. Thanks in advance, and hope y'all have a lovely day!!

ashley-hh avatar Nov 02 '21 06:11 ashley-hh

Thanks for the report, could you provide a minimal code example to reproduce this error?

ryanking13 avatar Nov 02 '21 07:11 ryanking13

We still don't fully support Safari: we don't run Safari tests in the continuous integration and I don't think any of the core devs use macs so we don't test things locally in Safari. I frequently do manual experiments in both Firefox and Chrome but not in Safari. So it may be a long time before issues like this are addressed.

hoodmane avatar Nov 02 '21 16:11 hoodmane

use macs so we don't test things locally in Safari

Actually once https://github.com/pyodide/pyodide/pull/1912 is resolved, we could potentially run Safari in CI. It adds more maintenance work, but then we do have recurrent requests to for better Safari support as well...

rth avatar Nov 02 '21 16:11 rth

Thanks for the report, could you provide a minimal code example to reproduce this error?

Thank you for your prompt response!! I believe this should do it:

Screen Shot 2021-11-02 at 3 01 06 PM

I'm using Version 14.1.2 of Safari.

ashley-hh avatar Nov 02 '21 19:11 ashley-hh

Thanks @ashley-hh ! Could you please copy paste it as a code snippet so that we wouldn't have to re-type it manually?

rth avatar Nov 02 '21 19:11 rth

Oh, of course! My bad -- here you go!

const script = `
import micropip
import numpy as np
import os
os.environ['MPLBACKEND'] = 'AGG'
import io, base64
import sys
sys.setrecursionlimit(1000)
await micropip.install('seaborn==0.9.0')
import seaborn as sns

data = np.asarray([[1, 0, .5], [1, 0, .5], [1, 0, .5]])
fig = sns.clustermap(data)

buf = io.BytesIO()
fig.savefig(buf, format='png')`

await self.pyodide.loadPackagesFromImports(script);
const res = await self.pyodide.runPythonAsync(script);

ashley-hh avatar Nov 02 '21 20:11 ashley-hh

Thanks! I can reproduce on Safari 14.1. As far as I can tell the error happens when calling sns.clustermap, even without the savefig part.

BTW, in Safari 13.1 I get,

Error: WebAssembly function with an i64 argument can't be called from JavaScript (
      evaluating `dynCall("viiii", index, [a1, a2, a3, a4])`

The error is likely different because 14.1 got BigInt support. So far not much ideas of how to fix it, but clearly some argument is not being converted while it should be, since we are building without WASM_BIGINT as far as I know.

rth avatar Nov 03 '21 20:11 rth

The flag WASM_BIGINT says that BigUint64Array and BigInt64Array exist. Emscripten always assumes that BigInt exists.

hoodmane avatar Nov 03 '21 21:11 hoodmane

@hoodmane Are you sure? https://emscripten.org/docs/getting_started/FAQ.html#how-do-i-pass-int64-t-and-uint64-t-values-from-js-into-wasm-functions doesn't sound like it.

rth avatar Nov 04 '21 11:11 rth

I think so. For a function with an i64 argument, dynCall always gets a BigInt. But to load or store BigInt into the wasm memory without BigUint64Array requires some bitwise operations. See here in my libffi port:

https://github.com/hoodmane/libffi-emscripten/blob/1fd79801595a43e26b65b59030a9040ba124ee24/src/wasm32/ffi.c#L51]

Both with and without WASM_BIGINT, we are expected to handle BigInt but without it HEAPU64 doesn't exist. Without BigInt support at all, it's just impossible to call functions with an i64 argument from Javascript as the error message says.

hoodmane avatar Nov 04 '21 15:11 hoodmane

It's funny, grepping around the emscripten source, I don't find much evidence for my claim that WASM_BIGINT works this way, but the libffi tests pass when built without WASM_BIGINT so that makes me think it's probably true anyways.

hoodmane avatar Nov 04 '21 15:11 hoodmane