Skip to content

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

Merged
jdtournier merged 1 commit intofix_reslice_adapter_for_complexfrom
reslice_complex_alternative
May 2, 2024
Merged

Adapter::Reslice: Alternative resolution of complex summation#2785
jdtournier merged 1 commit intofix_reslice_adapter_for_complexfrom
reslice_complex_alternative

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 requested a review from jdtournier January 23, 2024 03:46
@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
@Lestropie Lestropie restored the reslice_complex_alternative branch August 26, 2025 08:11
@Lestropie Lestropie deleted the reslice_complex_alternative branch August 27, 2025 00:08
@Lestropie Lestropie mentioned this pull request Sep 2, 2025
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