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

Added isclose test for std::complex<float> #2751

Closed

Conversation

nathan-mxr
Copy link

Added test demonstrating compiler error from isclose on xarray<std:;complex> as per request on #2730

Compiler:
Microsoft (R) C/C++ Optimizing Compiler Version 19.37.32825 for x64

Error:
C2678 binary '+': no operator found which takes a left-hand operand of type 'std::complex' (or there is no acceptable conversion) xtl\xtype_traits.hpp 94
C2672 'std::max': no matching overloaded function found xtensor\include\xtensor\xmath.hpp 1760
C3536 'd': cannot be used before it is initialized xtensor\include\xtensor\xmath.hpp 1758

@tdegeus
Copy link
Member

tdegeus commented Dec 4, 2023

Thanks! To me, it appears that there is some (implicit) conversion to std::complex<double> somewhere, and it appears that adding std::complex<float> and std::complex<double> might not be permitted by the standard library. Did you debug this already? I.e. do you know where things go south exactly?

@spectre-ns
Copy link
Contributor

spectre-ns commented Dec 7, 2023

@tdegeus

This compiler explorer shows the issue: https://godbolt.org/z/h34KoWKTn. This is the problematic code block where we try to add complex<float> with a complex<double> to determine the resulting type.

    template <class T0, class... REST>
    struct promote_type<T0, REST...>
    {
        using type = decltype(std::declval<std::decay_t<T0>>() + std::declval<typename promote_type<REST...>::type>());
    };

I'm not sure this makes sense to use complex numbers with isclose. Is close could use the difference of the magnitudes or it could be the Euclidian distance between the them which would be |a - b| where a and b are both complex.

Edit: I see now this is specifically for mixed arithmetic so I retract my previous thought on not using the operator+ for evaluating the resulting types. The fundemental issue here is that adding complex float with complex doubles is not valid in the STL

@spectre-ns
Copy link
Contributor

@nathan-mxr @tdegeus

I think adding this fix to the xtl will fix the issue:

    template <class T0, class T1, class... REST>
    struct promote_type<std::complex<T0>, std::complex<T1>, REST...>
    {
        using type = std::complex<typename promote_type<T0, T1, REST...>::type>;
    };

Thanks!

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.

3 participants