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

Python bindings should preserve float32 scalar Exprs #8420

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

steven-johnson
Copy link
Contributor

@steven-johnson steven-johnson commented Sep 16, 2024

Tweak the PyBind11 code so that float32 scalars created via numpy.float32() don't get converted into boolean expressions.

@runenordmo @dragly Please try this out with your local codebases and let me know if it solves the problem (and/or doesn't introduce new, worse problems) -- I don't want to land this without your feedback.

Fixes #8414

Tweak the PyBind11 code so that float32 scalars created via numpy.float32() don't get converted into boolean expressions.
@steven-johnson steven-johnson changed the title Python bindings should preserve float32 scalar Exprs (fixed #8414) Python bindings should preserve float32 scalar Exprs (fixes #8414) Sep 16, 2024
@runenordmo
Copy link

Thanks for looking at this, @steven-johnson 👍
We'll try in our code base and look at the changes tomorrow

@alexreinking alexreinking changed the title Python bindings should preserve float32 scalar Exprs (fixes #8414) Python bindings should preserve float32 scalar Exprs Sep 17, 2024
@steven-johnson
Copy link
Contributor Author

EDIT: looks like this fix is somewhat nondeterministic -- it fails for at least two targets for no obvious reason I can tell. I'll have to do some more debugging on this before it can land, but it's clearly not right as-is.

@dragly
Copy link
Contributor

dragly commented Sep 18, 2024

@steven-johnson it took a bit more time for us to get this tested (we are on a fork of an old Halide release, but backporting the change was no problem). We have run the new version on our end now and it fixes the problem. All tests are, so it seems to be okay in our code.

Let us know if you want to test again with a new version that has the necessary changes to pass your tests too.

@steven-johnson
Copy link
Contributor Author

Yeah, this fix is nondeterministic (by compiler/OS/etc) so it makes me too nervous to land. I don't have a better fix yet and I'm off the rest of the week so a better fix might not be on the way right now.

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

Successfully merging this pull request may close these issues.

Expr constructed with numpy float32 argument gets type halide bool or int32 in Python bindings
3 participants