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

Adapter::Reslice: Alternative resolution of complex summation #2785

Merged

Conversation

Lestropie
Copy link
Member

@jdtournier: This part of #2768 struck me as odd:

using summing_type = typename std::conditional<std::is_arithmetic<value_type>::value, double, cdouble>::type;

While it might result in appropriate resolution for template types currently compiled, it would attempt to use cdouble for any unexpected type. There's already a struct MR::is_complex<>, which makes more sense to me; its use makes the code intention far more clear IMO.

Uses an alternative design to f97d3ca / #2768 for supporting complex numbers in the reslice adapter. Specifically in determining the type to use during summation, f97d3ca chooses complex double if the type is not arithmetic, which could hypothetically lead to unexpected behaviour with unexpected template types. This alternative instead utilises the pre-existing is_complex<> SFINAE capability.
@Lestropie Lestropie self-assigned this Jan 23, 2024
@jdtournier jdtournier merged commit 88c9367 into fix_reslice_adapter_for_complex May 2, 2024
@jdtournier jdtournier deleted the reslice_complex_alternative branch May 2, 2024 12:18
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.

2 participants