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

Add optional NORDIC denoising step #3308

Open
tsalo opened this issue Jun 17, 2024 · 14 comments · May be fixed by #3395
Open

Add optional NORDIC denoising step #3308

tsalo opened this issue Jun 17, 2024 · 14 comments · May be fixed by #3395
Labels

Comments

@tsalo
Copy link
Collaborator

tsalo commented Jun 17, 2024

What would you like to see added in fMRIPrep?

If possible, it would be great to support NORDIC denoising.

NORDIC is currently written in MATLAB, which might be a problem.

Do you have any interest in helping implement the feature?

Yes

Additional information / screenshots

BIDS now supports separate noRF scans for the no-excitation pulse volumes people often acquire at the ends of their runs for NORDIC.

@tsalo tsalo added the feature label Jun 17, 2024
@effigies
Copy link
Member

Would be good to know how NORDIC fits into the fit and transform workflows. Does it need to be done before/after HMC/STC? Is it deterministic/nondeterministic? What's the runtime? Is there a compact set of parameters that can be saved, or is it basically just the noRF files are the parameters?

@tsalo
Copy link
Collaborator Author

tsalo commented Jun 17, 2024

Pinging @madisoth for his thoughts, but my understanding is that it needs to be done before HMC, STC, or basically anything else. I don't think it's deterministic, unfortunately.

Most parameters are determined from the data (e.g., how many noise volumes, whether there's phase data or not). The only one I can think of that users might want to vary is phase_filter_width, but there's a strong recommendation to use a value of 10 for fMRI data, so even that could be hardcoded.

Not sure about the runtime since I've only ever run it on large numbers of runs in jobs.

@mvdoc
Copy link
Contributor

mvdoc commented Jun 18, 2024

Just popping in to say that re the MATLAB implementation, I recently found this package with a python implementation of NORDIC: https://paquiteau.github.io/patch-denoising/ (and associated paper: https://hal.science/hal-03895194/file/isbi2023_denoise.pdf)

I did not have a chance to try it myself (or NORDIC itself), so buyer beware.

@tsalo
Copy link
Collaborator Author

tsalo commented Jun 19, 2024

Thanks @mvdoc! The repo looks really promising, and even if it's not completely up-to-date with SteenMoeller/NORDIC_Raw (which it very much could be), it would still be a great jumping off point.

I also wanted to note that, for multi-echo data, NORDIC could be run on each echo independently (which I believe is how most folks do it), or we could try the HYDRA-OC approach from this OHBM abstract by Marco Flores-Coronado and Cesar Caballero-Gaudes. In that approach, it looks like you concatenate the multi-echo in a spatial direction (Z in their case), run NORDIC, and then split up the denoised data.

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 5, 2024

I looked at the patch-denoising code and it doesn't seem like it uses noRF scans for its NORDIC implementation, so I'm not sure how close it is to the "official" method.

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 8, 2024

I think we might need to use the MATLAB version.

@Remi-Gau do you know any tricks for using MATLAB in containers/BIDS apps?

@Remi-Gau
Copy link
Contributor

Remi-Gau commented Aug 8, 2024

Yeah. Make sure the code is Octave compatible and use Octave.
Otherwise you are down the road of compiling Matlab code...
Where is the Nordic code base?
If it's the one I am thinking of I may have "opinions" about it...

@tsalo
Copy link
Collaborator Author

tsalo commented Aug 8, 2024

This one is the one I was planning to use, since it's the one others seem to be using. I don't know if it's Octave-compatible though. It does rely on the nifti/20130306 libary.

@tsalo
Copy link
Collaborator Author

tsalo commented Sep 17, 2024

It looks like someone has been working on Nipype interfaces for NORDIC (SteenMoeller/NORDIC_Raw#21).

@tsalo
Copy link
Collaborator Author

tsalo commented Sep 18, 2024

@MoniDoerig have you had a chance to test out the NORDIC Nipype interfaces you wrote in your PR? Do you know what else might need to go into the Docker image to make it work?

@MoniDoerig
Copy link

@tsalo Yes, I've tested the Nipype interface for the NIFTI_NORDIC script both on lab and open-source data, specifically 7T event-related fMRI data. The tests were conducted on Neurodesk Version 24-04-04.

You can find a Jupyter Notebook showcasing the usage and changes I made to the NIFTI_NORDIC script to make it compatible with Nipype, along with the Nipype interface itself here.

The Nipype interface is specifically for NIFTI_NORDIC, which, unlike the original NORDIC, does not require a noise-scan or the g-factor map. Also, it uses additional parmeters that can be adjusted. If no additional parameters are provided, the interface uses the default values. These could be hard-coded (e.g., using magnitude images only, specifying output format) to simplify the interface and remove reliance on an additional parameter struct. However, this would reduce flexibility in case other configurations are needed.

Regarding the Docker image:

  • It depends on MATLAB (tested with version R2022a, although I haven't tested MATLAB Runtime yet)
  • It also requires Nipype (tested with version 1.8.6) and the Python library scipy (tested with version 1.13.0) used for initializing the arguments struct in Python.

@tsalo
Copy link
Collaborator Author

tsalo commented Sep 23, 2024

Thank you @MoniDoerig. It sounds like the best move forward on fMRIPrep's end would be to fork NORDIC_Raw with your changes, compile it into a standalone application with MATLAB Runtime, release that so it can be downloaded easily, and then add it to the fMRIPrep Dockerfile. @effigies does that sound right to you?

@bpinsard
Copy link
Collaborator

bpinsard commented Oct 1, 2024

When I created an image for NORDIC with MCR it ended up being very large, not sure if that was because I had a licence with all the extensions. I don't think there are not many options to make MCR images much lighter, I tried to track all files used when running the MCR compiled files and removing the one unnecessary but it broke the MCR. Not sure if we want to make fMRIPrep docker images that heavier. And maybe we should try to work with the python implementation above that I just found about here.

@tsalo
Copy link
Collaborator Author

tsalo commented Oct 2, 2024

Agreed. @Remi-Gau reached out to the Python version's author and he's willing to work through bugs and such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants