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

[runenordmo]

With halide-19.0.0.dev36, a numpy.float32(1.2) as an argument to halide.Expr's constructor will give a resulting Expr of type hl.Bool():

>>> hl.Expr(np.float32(4.5))
<halide.Expr of type bool: (uint1)1>

Earlier versions like halide 13 also give unexpected results: <halide.Expr of type int32: 4>.

I'm not sure if this is an unintended use case or unintended behavior.
I initially discovered it because np.float32(np.pi) was part of a calculation and it ended up being treated as an integer, giving wrong results.

I would be greatful if you have any ideas to fix or avoid this type of behavior.
(Short term, we are keeping away from numpy.float32)

---

Here is a minimal example/test:

import halide as hl
import numpy as np

assert (hl.Expr(np.float32(1.2))).type() == hl.Float(32) # Error, currently gives hl.Bool() (or hl.Int(32))

[runenordmo]

Here is a more relevant minimal example as well

import halide as hl
import numpy as np

x = hl.Var("x")
result = hl.Func("result")
result[x] = x + hl.Expr(np.float32(np.pi))
output = result.realize([1])

# Fails: ACTUAL: 1 (or ACTUAL 3 with halide-13)
np.testing.assert_almost_equal(output[0], 3.1416, decimal=2) 

[dragly]

If I rearrange the following

Halide/python_bindings/src/halide/halide_/PyExpr.cpp

Lines 26 to 37 in f658eec

	.def(py::init([](bool b) {
	return Internal::make_bool(b);
	}))
	// PyBind11 searches in declared order,
	// int should be tried before float conversion
	.def(py::init<int>())
	.def(py::init<int64_t>())
	// Python float is implemented by double
	// But Halide prohibits implicitly construct by double.
	.def(py::init([](double v) {
	return double_to_expr_check(v);
	}))

into moving the double overload before bool overload

            // Python float is implemented by double
            // But Halide prohibits implicitly construct by double.
            .def(py::init([](double v) {
                return double_to_expr_check(v);
            }))
            .def(py::init([](bool b) {
                return Internal::make_bool(b);
            }))
            // PyBind11 searches in declared order,
            // int should be tried before float conversion
            .def(py::init<int>())
            .def(py::init<int64_t>())

then the issue goes away. I rebuilt the Python bindings and ran the above minimal example.

Based on this comment, it seems like the order is intentional:

// int should be tried before float conversion

Reading the pybind11 docs it seems like PyExpr is missing an explicit overload for np.float32, but I have not yet figured out what that explicit overload should be. Attempting to add py::float_ failed with a compiler error.

I am also a bit confused by the comment in the code when I compare that to what the pybind11 docs states. According to the docs, pybind11 runs resolution order in two passes. One where it attempts to match explicitly, then a second where it chooses the first one that matches implicitly.

It seems like we are hitting this issue in the second pass, where bool is preferred as an implicit target over double. But I do not understand why bool and int would be preferred over double in an implicit pass. It sounds like it would easily be a narrowing conversion. Perhaps except in the case of something that is int64 that does not trigger the explicit conversion.

@steven-johnson: It seems like you wrote the initial comment. Do you remember why the integers are preferred? The comment is from 2017, so I completely understand if you do not remember 😃

[shoaibkamil]

This PR for pybind11 seems related: pybind/pybind11#2060

@dragly Does this link give enough info on how we can resolve the issue?

[steven-johnson]

Do you remember why the integers are preferred? The comment is from 2017, so I completely understand if you do not remember

Sadly, yeah, I spun down those brain cells a while ago. The current behavior definitely seems like a bug that needs fixing though! I wonder if just adding an overload for float would work?

[steven-johnson]

@alexreinking FYI we should try to fix this prior to the Halide 19 release

[alexreinking]

LLVM 19.1.0 is due tomorrow and we normally trail them by a week or two anyway

[dragly]

This PR for pybind11 seems related: pybind/pybind11#2060

@dragly Does this link give enough info on how we can resolve the issue?

I have not been able to find a solution based on that link yet. The linked PR seems to add new types to pybind11 that could help solve the issue, but I am not sure it helps if those are not in pybind11 already. Maybe there is something in there if we try to mimick the behavior the PR implements, though.

Do you remember why the integers are preferred? The comment is from 2017, so I completely understand if you do not remember

Sadly, yeah, I spun down those brain cells a while ago. The current behavior definitely seems like a bug that needs fixing though! I wonder if just adding an overload for float would work?

I tried that, but for some reason float does not seem to be picked up as an explicit overload equivalent to np.float32.

Reordering to have double before bool and int fixes the issue, but I am not sure if that also changes behavior elsewhere. It seems to fix it by catching np.float32 as an implicit conversion to double in the second pass. The same goes for float if it comes before bool or int in the chain of overloads.

[steven-johnson]

PTAL at #8420 -- I very much want feedback from @dragly and @runenordmo before landing that change