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

[feature] add subspace projection model. #201

Merged
merged 10 commits into from
Oct 7, 2024

Conversation

mcencini
Copy link
Contributor

@mcencini mcencini commented Oct 1, 2024

This PR add a new extension to enable low rank subspace projected NUFFT as described (in a Cartesian context) here. A non-exhaustive list potential applications is:

  • Multi-contrast MRI with virtual echoes (e.g., Tamir et al.)
  • Acceleration of iterative reconstruction for quantitative MRI (e.g., Magnetic Resonance Fingerprinting, MR Multitasking)
  • Dynamic (e.g., cardiac) MRI

examples/example_subspace.py Outdated Show resolved Hide resolved
@mcencini mcencini requested a review from philouc October 1, 2024 18:52
Copy link
Collaborator

@philouc philouc left a comment

Choose a reason for hiding this comment

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

Minor typo left.

examples/example_subspace.py Outdated Show resolved Hide resolved
@mcencini mcencini requested a review from philouc October 2, 2024 04:54
docs/nufft.rst Outdated Show resolved Hide resolved
docs/nufft.rst Outdated Show resolved Hide resolved
examples/example_subspace.py Outdated Show resolved Hide resolved
# installable using ``pip install brainweb-dl``.

from mrinufft.extras import get_brainweb_map

Copy link
Collaborator

Choose a reason for hiding this comment

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

slice selection, 90

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure I got this - do you think we should add a comment specifying we are selecting the 90-th slice?

Copy link
Collaborator

Choose a reason for hiding this comment

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

just slice selection for 2D processing

examples/example_subspace.py Outdated Show resolved Hide resolved
src/mrinufft/extras/field_map.py Outdated Show resolved Hide resolved
src/mrinufft/extras/sim.py Outdated Show resolved Hide resolved
src/mrinufft/operators/subspace.py Outdated Show resolved Hide resolved
src/mrinufft/operators/subspace.py Outdated Show resolved Hide resolved
src/mrinufft/operators/subspace.py Outdated Show resolved Hide resolved
Copy link
Member

@paquiteau paquiteau left a comment

Choose a reason for hiding this comment

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

Hello Matteo, Thanks for this PR, having the test examples and documentation altogether really helps to grasp the new feature.

I have some minors comments + I would advise to have the dimension ordered as B,T,C,... as the batch dimension is for independent data points. (but if it makes more sense for the computation to be done in T,B,C... you can just move the axes around)

examples/example_subspace.py Outdated Show resolved Hide resolved
src/mrinufft/extras/field_map.py Outdated Show resolved Hide resolved
src/mrinufft/extras/field_map.py Outdated Show resolved Hide resolved
src/mrinufft/operators/subspace.py Outdated Show resolved Hide resolved
@mcencini mcencini requested review from philouc and paquiteau October 3, 2024 02:09
@mcencini
Copy link
Contributor Author

mcencini commented Oct 7, 2024

I am not sure why tests would fail for gpunufft density compensation - I did not touch anything from that subpackage

@paquiteau
Copy link
Member

The CI is just a bit flaky it seems.

@paquiteau paquiteau merged commit 0037902 into mind-inria:master Oct 7, 2024
7 of 12 checks passed
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