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

Weird float arg bug on MicroPython when calling a JS method #1926

Closed
3 tasks done
GoToLoop opened this issue Jan 6, 2024 · 11 comments
Closed
3 tasks done

Weird float arg bug on MicroPython when calling a JS method #1926

GoToLoop opened this issue Jan 6, 2024 · 11 comments
Labels
needs-triage Issue needs triage type: bug Something isn't working upstream Issues relating to upstream features or bugs.

Comments

@GoToLoop
Copy link

GoToLoop commented Jan 6, 2024

Checklist

  • I added a descriptive title
  • I searched for other issues and couldn't find a solution or duplication
  • I already searched in Google and didn't find any good information or help

What happened?

On file "ball.py" from this project link:
https://PyScript.com/@gotoloop/bouncing-colorful-balls
https://GoToLoop.PyScriptApps.com/bouncing-colorful-balls/latest/mpy.html

This statement below produces an unexpected value for JS property p5.Vector.y:
vel.set( p.random(-VEL, VEL), p.random(-VEL, VEL) )

Inspecting both props. x & y, former is OK, latter is buggy:
print('Bug:', vel.x, vel.y) # vel.x OK. vel.y either 0 or close to 0

These are my 2 workarounds:

vel.y = p.random(-VEL, VEL) # fix A for µPython: change prop y
print('Fix A:', vel.y)

vel.set( p.random(-VEL, VEL), p.random(-VEL, VEL), 0 ) # fix B: 3 args
print('Fix B:', vel.y, '\n')

Links to p5.Vector.set()'s reference & implementation:
https://p5js.org/reference/#/p5.Vector/set
https://GitHub.com/processing/p5.js/blob/v1.9.0/src/math/p5.Vector.js#L194-L215

Basically, the p5.Vector.y bug happens when we pass 2 float arguments to p5.Vector.set().

Here's a table describing all calling combinations for p5.Vector.set(x, y, z):

set() y Status
3 args OK
1st arg is int OK
2nd arg is int OK
(float, float) BUG

Changing to Pyodide on "mpy.html" fixes that bug:
<script type=py src=mpy.py config=mpy.toml></script>

Although it creates another 1, which is fixed by wrapping callback reset() w/ create_proxy().

This float bug is so bizarre that I fear it's almost unfixable!

What browsers are you seeing the problem on? (if applicable)

64-bit Win10 Chromium-based [120.0.6099.18] SlimJet [42.0.1.0]

Console info

Bug: -0.5395090559185229 0
Fix A: -1.482236302859048
Fix B: 1.286516784117789

Bug: 1.119370978488424 -1.810775189542148e-255
Fix A: -1.924031271160342
Fix B: 1.945697956813783

Bug: 0.7756889052130456 -1.235862254512358e-260
Fix A: -0.2756648610788321
Fix B: -0.9008514572086463

Bug: -0.7191254625197807 0
Fix A: 0.8282101963304846
Fix B: 1.679441867246878

Bug: -1.716702460120417 0
Fix A: 1.604368563489844
Fix B: 0.09771180745833608

Bug: -0.8124285914132443 3.64391778535868e-06
Fix A: 1.087312791527494
Fix B: -0.4058443571727768

Bug: 0.3558254512385002 8.559726805063262e-169
Fix A: 0.9826754205929848
Fix B: -0.93238694267691

Bug: 1.482914407715557 0
Fix A: -0.6981715094363947
Fix B: -1.322550776350354

Bug: 1.323650746565667 0
Fix A: -0.5335516308597006
Fix B: 0.05895170364000801

Bug: 0.5508805440391402 2.822373730176611e-19
Fix A: -1.647803583511953
Fix B: 0.667917896839481

Bug: -0.3260353931481585 -1.536602405035822e-242
Fix A: 0.5654981230798706
Fix B: -1.828681501138143

Bug: 0.03404493958355381 -8.934472225291866e-173
Fix A: 0.01703001733473997
Fix B: 1.793174117626485

Bug: 1.626620928731301 -8.438834011474368e-103
Fix A: -1.270730248110303
Fix B: 0.6357773505125817

Bug: -0.0273815814903644 0
Fix A: 0.7190692070353073
Fix B: -1.856428010203058

Bug: 0.5207591223191619 1.184032180198601e-265
Fix A: -0.1246307488426179
Fix B: -0.8210687637135106

Additional Context

No response

@GoToLoop GoToLoop added needs-triage Issue needs triage type: bug Something isn't working labels Jan 6, 2024
@WebReflection
Copy link
Contributor

This float bug is so bizarre that I fear it's almost unfixable!

floats have been fixed recently in MicroPython but this is definitively a MicroPython issue.

I am pinging @dpgeorge here as from our side there's not much we can do.

As the linked live demo does not work for me (at all) it would be super nice if you could present just the minimal code that breaks, without involving any 3rd party library, something like a funciton that with 2 floats has issues ... that would surely help fixing the issue because ... well, all issues can be fixed, no need to be pessimistic 😉

@GoToLoop
Copy link
Author

GoToLoop commented Jan 8, 2024

As the linked live demo does not work for me (at all) it would be super nice if you could present just the minimal code that breaks, without involving any 3rd party library,

I'm perplexed why my demo isn't working for you!

That "glitch" only shows up when invoking that p5.Vector:set() method from that 3rd party library passing exactly 2 float arguments!

Any other combination won't trigger that "very close to 0" bug.

@dpgeorge
Copy link

dpgeorge commented Jan 8, 2024

Thanks for the ping. I've managed to reproduce the issue with a simple js.console.log(x, x), where x is a floating point number (on the Python side). I'll try to get a fix shortly.

@dpgeorge
Copy link

dpgeorge commented Jan 8, 2024

Should be fixed in the latest version here: https://www.npmjs.com/package/@micropython/micropython-webassembly-pyscript/v/1.21.0-279

@GoToLoop
Copy link
Author

GoToLoop commented Jan 8, 2024

For historical reasons, I'm gonna leave 2 snapshots of my project showing the "glitch".

It turns out, the current recommended version https://PyScript.net/releases/2023.11.1/core.js from https://PyScript.net is even worse, corrupting both float arguments when invoking a JS function:
https://PyScript.com/@gotoloop/bouncing-colorful-balls/v1
https://GoToLoop.PyScriptApps.com/bouncing-colorful-balls/v1/mpy.html

My actual version comes from https://Unpkg.com/@PyScript/core, which always gets the latest.

But I'm also gonna leave my 2nd snapshot using https://PyScript.net/releases/2024.1.1/core.js which corrupts only the 2nd float arg when passing 2 float args:
https://PyScript.com/@gotoloop/bouncing-colorful-balls/v2
https://GoToLoop.PyScriptApps.com/bouncing-colorful-balls/v2/mpy.html

@WebReflection
Copy link
Contributor

WebReflection commented Jan 8, 2024

I am going to update MicroPython in polyscript and bring that latest to PyScript too ... I will mention this issue to the team so that we can hopefully officially release a version that works sooner than later.

Pinging @ntoll here too.

@WebReflection
Copy link
Contributor

WebReflection commented Jan 8, 2024

@GoToLoop

I'm perplexed why my demo isn't working for you!

this is why:

Screenshot from 2024-01-08 15-23-15

works on Chrome, maybe Firefox (I haven't tried), fails on Safari / WebKit.

As mentioned elsewhere, mime-type matters on the Web 😉

edit re-tested and now everything seems to be fine, even on WebKit, maybe a cache issue or maybe you changed something ... still it was a mime-type gotcha.

@GoToLoop
Copy link
Author

GoToLoop commented Jan 8, 2024

or maybe you changed something ...

The only file I've edited since opening this GH issue was "mpy.html" just a few hours, and only to add 2023.11.1 & 2024.1.1 <script> tags and snapshotting them as v1 & v2 respectively.

Anyways, thx a lot every1 for taking care of this mysterious Python-to-JS 2 float args behavior! 😀

@ntoll
Copy link
Member

ntoll commented Jan 10, 2024

Happy to cut a release with this update. Please give me a 👍 @WebReflection and I'll make it so.

@WebReflection
Copy link
Contributor

@ntoll I'd like to have the currentScript story fixed in before ... otherwise I need to at least update polyscript to its latest as more got fixed in the meanwhile (config in workers as JSON not working)

@WebReflection WebReflection added 3rd party Errors coming from foreign projects we either enable or rely on upstream Issues relating to upstream features or bugs. and removed 3rd party Errors coming from foreign projects we either enable or rely on labels Jan 11, 2024
@WebReflection
Copy link
Contributor

The latest MicroPython has landed officially so this bug should be solved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Issue needs triage type: bug Something isn't working upstream Issues relating to upstream features or bugs.
Projects
None yet
Development

No branches or pull requests

4 participants