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

Reslice adapter: allow use with complex numbers #2768

Merged
merged 4 commits into from
May 9, 2024

Conversation

jdtournier
Copy link
Member

@jdtournier jdtournier commented Dec 13, 2023

Simple fix to enable Adapter::Reslice to work as expected. Required for complex number support in mrpeek (PR: MRtrix3/mrpeek#47).

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
Copy link
Member

Lestropie commented Mar 5, 2024

@jdtournier
Copy link
Member Author

closed in favour of #2785.

@jdtournier jdtournier closed this May 2, 2024
@jdtournier jdtournier reopened this May 2, 2024
Adapter::Reslice: Alternative resolution of complex summation
@jdtournier
Copy link
Member Author

OK, not closed, but merged #2785 in here as that's how it was set up...

@Lestropie Lestropie requested a review from a team May 9, 2024 11:01
Copy link
Member

@Lestropie Lestropie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub might accept my review on this one; depends on whether it considers your merge commit, or my commit, to be "the most recent"...

@Lestropie Lestropie added this pull request to the merge queue May 9, 2024
Merged via the queue into master with commit c52504b May 9, 2024
5 checks passed
@jdtournier jdtournier deleted the fix_reslice_adapter_for_complex branch May 9, 2024 11:33
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