Float Overloads Specialize to Least Precision

[ax3l]

Issue description

When overloading for float parameters, such as:

    m.def("bar", [](float const& value) {
        std::cout << "float! " << value << std::endl;
    });
    m.def("bar", [](double const& value) {
        std::cout << "double! " << value << std::endl;
    });
    m.def("bar", [](long double const& value) {
        std::cout << "long double! " << value << std::endl;
    });

the corresponding call from a python builtin.float will always select the least precise floating point type:

from cmake_example import bar

bar(1.23)
# float! 1.23

type(1.23)
# float

bar(1.23e38)
# float! 1.23e+38

type(1.23e38)
# float

bar(1.23e39)
# float! inf

type(1.23e39)
# float

Reproducible example code

See above.

Contrary, in Boost.Python always the most precise floating point type was selected:

void bar1( float const& value ) {
    std::cout << "float! " << value << std::endl;
}
void bar2( double const& value ) {
    std::cout << "double! " << value << std::endl;
}
void bar3( long double const& value ) {
    std::cout << "longdouble! " << value << std::endl;
}

// ...

    def("bar", bar1);
    def("bar", bar2);
    def("bar", bar3);

# ...

bar(1.23)
# longdouble! 1.23

Proposed change

besides that making all three overloads possible for builtin.float but only selecting one is still fine, we should change the priorities of the selected overloads similar to the handling in Boost.Python to the largest/most-precise type.

for explicit control of passing float32/64/128 we should fix the bug reported in #1224

[ax3l]

we should change the priorities of the selected overloads

work-around for pybind11: change the order of definitions to

    m.def("bar", [](long double const& value) {
        std::cout << "long double! " << value << std::endl;
    });
    m.def("bar", [](double const& value) {
        std::cout << "double! " << value << std::endl;
    });
    m.def("bar", [](float const& value) {
        std::cout << "float! " << value << std::endl;
    });

We should still order these overloads automatically improving predictability in more complex situations than the example shown here.

[YannickJadoul]

Python's floats are C++ doubles, right? So long double should never be matched?

But yes, it seems a bit weird. We could consider casting a Python float to a C++ float to be a "conversion", so it would only get converted on the second pass, but how much existing code would we then break?

[bstaletic]

With scalar types, pybind will always pick the first one it finds. Order matters a lot and long doubles are selected if encountered first.

[YannickJadoul]

That's the current situation, yes. But isn't it weird that a double gets narrowed to float without any warning?

If that's fine, we can close this issue, and maybe just make a note in the docs. Any further thoughts, 2 years later, @ax3l?

[bstaletic]

The same happens with integer types. A python long would happily match a short. It is weird and I don't like that it's the case, but I'm not sure how that can be fixed.

[YannickJadoul]

There has to be some kind of type trait that specifies if a conversion can be implicit/promoting, no? I quickly looked but couldn't immediately find it, though.

[bstaletic]

is_integral_v<T> && is_integral_v<U> && is_convertible_v<T, U> && ((is_signed_v<T> && is_signed_v<U>) || (is_unsigned_v<T> && is_unsigned_v<U>)) && sizeof(T) <= sizeof(U)

The above would be able to constrain integral types. Unfortunately, "integral" doesn't mean "integer" and matches all kinds of character types and even bools.

[YannickJadoul]

Both is_convertible_v<float, double> and is_convertible_v<double, float> are true, right? So that wouldn't help us.

Maybe we just need a note in the docs.

[bstaletic]

The is_convertible_v part was there to keep us from converting something stupid, like std::string to int. Interestingly Boost.Python gets this right. I honestly don't know what's the best option here.

[YannickJadoul]

(Sorry, I missed the final sizeof comparison)

[ax3l]

Re your question: I still use this order-dependent placement with bold warning doc strings in my code and it is confusing. Also don't know how boost python solves this, maybe @rwgk knows?

[bstaletic]

The more I delve through the stale issues the more issues I find that come down to "order-dependent placement", as @ax3l put it. Yesterday I was tempted to just close them all and point people at this issue.

[rwgk]

Hi @ax3l from very-long-ago memory / off the top of my head: possibly the secret is simply that Boost.Python tries the overloads in reverse order. At least many times I wrote my bindings with that assumption, manually ordering the overloads with that in mind. Do you still have the environment handy that you need for running your reproducer? What happens if you reverse the order in your Boost.Python example?
It could be that Boost.Python does something special for numerical values that I'm unaware of. The simple proposed experiment would tell us.

[YannickJadoul]

The more I delve through the stale issues the more issues I find that come down to "order-dependent placement"

The main cause is of course that you're adding something, i.e. overloads, to Python that it doesn't support by default. The only way to really solve this, is to implement something closer to the C++ overload resolution rules, but I don't think that has any chance of being straightforward enough to be an acceptable change?

[bstaletic]

Speaking as a user, I don't really care, since I'm not affected.

Speaking as a "collaborator", I don't think I want to implement best viable function according to C++ specs. Crazy, I know.

[rwgk]

We discussed this question for Boost.Python a few times and always ended up with @bstaletic 's conclusion. We just stuck with the simple approach to try the overloads in the order given, with an unobvious twist though: for constructors the overloads are searched from 1st def'ed to last def'ed. For functions and member functions it's the reverse, from last def'ed to 1st def'ed. I believe (!) that asymmetry was mostly by chance. Dave didn't want to change the behavior we somehow wound up with. What really matters though is that the order of the overload resolution is predictable / deterministic. Is that the case with pybind11?

[YannickJadoul]

What really matters though is that the order of the overload resolution is predictable / deterministic. Is that the case with pybind11?

As far as I know: yes. But it would be nice to confirm your hypothesis about @ax3l's use case in Boost.Python.

[ax3l]

I did the following to compare the two in a docker run -it ubuntu:18.04 container:

apt update
apt install -y cmake git g++ libboost-python-dev

git clone --recurse-submodules https://github.com/pybind/cmake_example.git
mkdir cmake_example/build

cat >cmake_example/src/main.cpp <<EOL
#include <pybind11/pybind11.h>
#include <iostream>

namespace py = pybind11;

PYBIND11_MODULE(cmake_example, m) {
    m.def("bar", [](float const& value) {
        std::cout << "float! " << value << std::endl;
    });
    m.def("bar", [](double const& value) {
        std::cout << "double! " << value << std::endl;
    });
    m.def("bar", [](long double const& value) {
        std::cout << "long double! " << value << std::endl;
    });
}
EOL

cat >cmake_example/tests/test.py <<EOL
from cmake_example import bar

bar(1.23)
type(1.23)

bar(1.23e38)
type(1.23e38)

bar(1.23e39)
type(1.23e39)
EOL

cd cmake_example/build
cmake ..
make

PYTHONPATH=$PWD python3 ../tests/test.py
# float! 1.23
# float! 1.23e+38
# float! inf

Now going on with Boost Python:

cat >../src/main.cpp <<EOL
#include <boost/python.hpp>
#include <iostream>

void bar1( float const& value ) {
    std::cout << "float! " << value << std::endl;
}
void bar2( double const& value ) {
    std::cout << "double! " << value << std::endl;
}
void bar3( long double const& value ) {
    std::cout << "longdouble! " << value << std::endl;
}

BOOST_PYTHON_MODULE(cmake_example)
{
    using namespace boost::python;
    def("bar", bar1);
    def("bar", bar2);
    def("bar", bar3);
}
EOL

g++ -I/usr/include/python3.6m  -fPIC -std=c++14 -o CMakeFiles/cmake_example.dir/src/main.cpp.o -c ../src/main.cpp -lboost_python3-py36
g++ -fPIC -shared  -o cmake_example.cpython-36m-x86_64-linux-gnu.so CMakeFiles/cmake_example.dir/src/main.cpp.o -lboost_python3-py36 -lpython3.6m

PYTHONPATH=$PWD python3 ../tests/test.py 
longdouble! 1.23
longdouble! 1.23e+38
longdouble! 1.23e+39

[ax3l]

If I turn in Boost.Python the module definition order around

    def("bar", bar3);
    def("bar", bar2);
    def("bar", bar1);

I get:

float! 1.23
float! 1.23e+38
float! inf

If I put double last:

    def("bar", bar1);
    def("bar", bar3);
    def("bar", bar2);

I get

double! 1.23
double! 1.23e+38
double! 1.23e+39

That seems to confirm what you wrote above, function overloads had last-to-first precedence in Boost.Python.

[ax3l]

I guess that solves the mystery and unless someone is super motivated I guess that @bstaletic 's

Speaking as a "collaborator", I don't think I want to implement best viable function according to C++ specs. Crazy, I know.

applies ^^

[YannickJadoul]

Thanks for confirming and checkin, @ax3l! :-)