Add 3D wave caipi #38
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Yay, thanks a lot for this !
Regards
Chaithya G R
…On Tue, 10 Oct 2023, 2:52 am Guillaume Daval-Frérot, < ***@***.***> wrote:
This PR adds the 3D Wave-CAIPI trajectory based on the work from Bilgic,
Berkin, Borjan A. Gagoski, Stephen F. Cauley, Audrey P. Fan, Jonathan R.
Polimeni, P. Ellen Grant, Lawrence L. Wald, and Kawin Setsompop.
"Wave‐CAIPI for highly accelerated 3D imaging." Magnetic resonance in
medicine 73, no. 6 (2015): 2152-2162. This implementation includes
additional features:
- a parameter to choose the packing method, defining how the different
helices should be positioned (i.e. following square or hexagonal tiling,
over concentric circles or randomly)
- a parameter to choose the shape covered over the kx-ky plane
(circle, square, diamond or anything defined through p-norms)
Hereafter is a simple rendering with a square tiling of a circle, shown in
3D and over the kx-ky 2D plane.
[image: image]
<https://user-images.githubusercontent.com/20557033/273699566-9ad47394-791d-4380-8238-91f22663917f.png>
[image: image]
<https://user-images.githubusercontent.com/20557033/273699619-46f0f59c-f298-4d16-96d9-89b76fbe0797.png>
------------------------------
You can view, comment on, or merge this pull request online at:
#38
Commit Summary
- 003b441
<003b441>
Add 3D wave caipi
File Changes
(2 files <https://github.com/mind-inria/mri-nufft/pull/38/files>)
- *M* src/mrinufft/trajectories/trajectory3D.py
<https://github.com/mind-inria/mri-nufft/pull/38/files#diff-439d40c06385e2982bedf92440cca8f89dfbc1d4e227f3178f9cc3e54981be92>
(68)
- *M* src/mrinufft/trajectories/utils.py
<https://github.com/mind-inria/mri-nufft/pull/38/files#diff-b1daaaba004de808aadd34c9c51a89db5aeb2201e48a6758f413d6198e89f576>
(13)
Patch Links:
- https://github.com/mind-inria/mri-nufft/pull/38.patch
- https://github.com/mind-inria/mri-nufft/pull/38.diff
—
Reply to this email directly, view it on GitHub
<#38>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBSZM65SW7G63BBGEMAPCTX6RTKJAVCNFSM6AAAAAA5ZLRCH6VHI2DSMVQWIX3LMV43ASLTON2WKOZRHEZTGOBUGE4DGNQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Impressive Guillaume! Many thanks |
|
Nice work ! I think we should also propose the canonical way of reconstructing data acquired with this trajectory (e.g FFTs and PSF deconvolution) even if its only using a dummy numpy backend, and this also open the possibility to compare it with a NUFFT based reconstruction. |
That's a great suggestion for a future PR. I don't personally plan to work on that soon, should we create an issue to keep that in mind ? |
paquiteau
requested changes
Oct 11, 2023
Comment on lines
242
to
252
| # Ruff doesn't like lambdas | ||
| def _sort_main(x): | ||
| return nl.norm(x, ord=order) | ||
|
|
||
| def _sort_tie(x): | ||
| return nl.norm(x, ord=tie_order) | ||
|
|
||
| positions = np.array(positions) * spacing | ||
| positions = sorted(positions, key=_sort_tie) | ||
| positions = sorted(positions, key=_sort_main) | ||
| positions = positions[:Nc] |
There was a problem hiding this comment.
A functools.partial will do the trick (needs to be imported on top though)
Suggested change
| # Ruff doesn't like lambdas | |
| def _sort_main(x): | |
| return nl.norm(x, ord=order) | |
| def _sort_tie(x): | |
| return nl.norm(x, ord=tie_order) | |
| positions = np.array(positions) * spacing | |
| positions = sorted(positions, key=_sort_tie) | |
| positions = sorted(positions, key=_sort_main) | |
| positions = positions[:Nc] | |
| positions = np.array(positions) * spacing | |
| positions = sorted(positions, key=partial(nl.norm, ord=order)) | |
| positions = sorted(positions, key=partial(nl.norm, ord=tie_order)) | |
| positions = positions[:Nc] |
src/mrinufft/trajectories/utils.py
Outdated
Comment on lines
599
to
608
| if not isinstance(shape, str): | ||
| return shape | ||
| elif shape in ["square"]: | ||
| return np.inf | ||
| elif shape in ["circle", "circular"]: | ||
| return 2 | ||
| elif shape in ["rhombus", "diamond"]: | ||
| return 1 | ||
| else: | ||
| raise NotImplementedError(f"Unknown shape name: {shape}") |
There was a problem hiding this comment.
do we really need the redundancy of "circle/circular" and "rhombus/diamon" ? a single possibility would leads to less ambiguity.
Suggested change
| if not isinstance(shape, str): | |
| return shape | |
| elif shape in ["square"]: | |
| return np.inf | |
| elif shape in ["circle", "circular"]: | |
| return 2 | |
| elif shape in ["rhombus", "diamond"]: | |
| return 1 | |
| else: | |
| raise NotImplementedError(f"Unknown shape name: {shape}") | |
| SHAPE = {"circle": 2, "square": np.inf, "diamond": 1} # an enum would be overkill | |
| if not isinstance(shape,str): | |
| return shape | |
| try: | |
| return SHAPE[shape] | |
| except KeyError as e: | |
| raise Value(f"Unknown shape name: {shape}") from e |
paquiteau
approved these changes
Oct 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the 3D Wave-CAIPI trajectory based on the work from Bilgic, Berkin, Borjan A. Gagoski, Stephen F. Cauley, Audrey P. Fan, Jonathan R. Polimeni, P. Ellen Grant, Lawrence L. Wald, and Kawin Setsompop. "Wave‐CAIPI for highly accelerated 3D imaging." Magnetic resonance in medicine 73, no. 6 (2015): 2152-2162. This implementation includes additional features:
Hereafter is a simple rendering with a square tiling of a circle, shown in 3D and over the kx-ky 2D plane.