Skip to content

mrdegibbs: update doc to soften warning about partial Fourier#2923

Merged
Lestropie merged 3 commits intomasterfrom
mrdegibbs_update_doc
Jun 20, 2024
Merged

mrdegibbs: update doc to soften warning about partial Fourier#2923
Lestropie merged 3 commits intomasterfrom
mrdegibbs_update_doc

Conversation

@jdtournier
Copy link
Member

@jdtournier jdtournier added this to the 3.1.0 updates milestone Jun 14, 2024
@jdtournier jdtournier requested a review from a team June 14, 2024 13:28
@jdtournier jdtournier self-assigned this Jun 14, 2024
@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

bjeurissen
bjeurissen previously approved these changes Jun 19, 2024
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.

  • Suggest having master as base branch

  • Currently a note about filtering is slid in at the last minute on a paragraph about partial Fourier. I would suggest a separate DESCRIPTION paragraph on disabling on-scanner filtering (both k-space-based and image-based).

@jdtournier jdtournier force-pushed the mrdegibbs_update_doc branch from c910e42 to 15e558f Compare June 19, 2024 14:03
@jdtournier jdtournier changed the base branch from dev to master June 19, 2024 14:04
@jdtournier jdtournier dismissed bjeurissen’s stale review June 19, 2024 14:04

The base branch was changed.

@jdtournier
Copy link
Member Author

OK, pull request redirected to master, and more explicit discussion of filtering, as requested. Here's the full text of the command description, for convenience - changes in bold at the end. Comments & suggestions welcome as always.

This application attempts to remove Gibbs ringing artefacts from MRI images using the method of local subvoxel-shifts proposed by Kellner et al. (see reference below for details).

This command is designed to run on data directly after it has been reconstructed by the scanner, before any interpolation of any kind has taken place. You should not run this command after any form of motion correction (e.g. not after dwifslpreproc). Similarly, if you intend running dwidenoise, you should run denoising before this command to not alter the noise structure, which would impact on dwidenoise's performance.

For best results, any form of filtering performed by the scanner should be disabled, whether performed in the image domain or k-space. This includes elliptic filtering and other filters that are often applied to reduce Gibbs ringing artifacts. While this method can still safely be applied to such data, some residual ringing artefacts may still be present in the output.

Note that this method is designed to work on images acquired with full k-space coverage. Running this method on partial Fourier (‘half-scan’) may not fully remove all ringing artifacts, and you may observe residuals of the original artifact in the partial-fourier direction. Nonetheless, application of the method is still safe and worthwhile. Users are however encouraged to acquired full-Fourier data where possible.

@Lestropie
Copy link
Member

Will require executing docs/generate_user_docs.sh also.

@jdtournier
Copy link
Member Author

@Lestropie, feel free to merge if you're OK with these last few changes.

@Lestropie Lestropie enabled auto-merge June 20, 2024 09:13
@Lestropie Lestropie added this pull request to the merge queue Jun 20, 2024
Merged via the queue into master with commit a53279c Jun 20, 2024
@Lestropie Lestropie deleted the mrdegibbs_update_doc branch June 20, 2024 10:04
@Lestropie Lestropie mentioned this pull request Jan 14, 2025
31 tasks
Lestropie added a commit that referenced this pull request Jan 15, 2025
Resolution of content merged to dev initially omitted from #3061.
- #2693 (completely omitted)
- #3005 (partial propagation / independent address of some common issues on both branches)
- #3001 (completely omitted)
- #2908 (completely omitted)
- #2955 (completely omitted)
- #2600 (completely omitted)
- #2962 (completely omitted)
- #2935 (completely omitted)
- #2923 (completely omitted)
- #2910 (completely omitted)
- #2638 (completely omitted)
- #2698 (completely omitted)
- #2721 (completely omitted)
- #2794 (completely omitted)
- #2768 (completely omitted; required modification to conform to other dev changes)
- #2713 (residual compilation errors following adf8fdd, including resolution against changes in #2437 on dev.
Lestropie added a commit that referenced this pull request Jan 15, 2025
Resolution of content merged to dev initially omitted from #3061.
- #2693 (completely omitted)
- #3005 (partial propagation / independent address of some common issues on both branches)
- #3001 (completely omitted)
- #2908 (completely omitted)
- #2955 (completely omitted)
- #2600 (completely omitted)
- #2962 (completely omitted)
- #2935 (completely omitted)
- #2923 (completely omitted)
- #2910 (completely omitted)
- #2638 (completely omitted)
- #2698 (completely omitted)
- #2721 (completely omitted)
- #2794 (completely omitted)
- #2768 (completely omitted; required modification to conform to other dev changes)
- #2713 (residual compilation errors following adf8fdd, including resolution against changes in #2437 on dev.
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants