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

include IVIM code from Dr. Amita Shukla-Dave Lab at MSKCC #52

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

locastre
Copy link

@locastre locastre commented Mar 4, 2024

We have contributed our codebase for intravoxel incoherent motion (IVIM) code from the MRI-QAMPER MATLAB package, developed by Dr. Amita Shukla-Dave's lab at Memorial Sloan Kettering Cancer Center.

Authors: Eve LoCastro ([email protected]), Dr. Ramesh Paudyal ([email protected]), Dr. Amita Shukla-Dave ([email protected])
Institution: Memorial Sloan Kettering Cancer Center
Department: Medical Physics
Address: 321 E 61st St, New York, NY 10022

@@ -0,0 +1,24 @@
Copyright (c) 2014, Jimmy Shen
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be included? It seems to be a separate toolbox. https://www.mathworks.com/matlabcentral/fileexchange/8797-tools-for-nifti-and-analyze-image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey! Thanks for this great submit!
Some questions:
It seems like the toolbox includes some generic toolboxes like the NIFTI toolbox. Do we need that to be part of the package, or could we take it as a requirement?
Also, what does the code do? Is it just fitting, or does it do more processing?

Thanks!

Oliver and Eric

@locastre
Copy link
Author

locastre commented Mar 6, 2024 via email

@locastre
Copy link
Author

locastre commented Mar 6, 2024 via email

@etpeterson
Copy link
Contributor

Just checking in on this because it's been a while. Were you planning on cleaning it or were you expecting someone to take this over?

@oliverchampion
Copy link
Collaborator

Hey! Way too late, but I am reviewing the code. You code is way more extensive then all the other commits, so this may take some more effort/time. But we will get there!
For now there are some things to address.

@oliverchampion
Copy link
Collaborator

You have a demo as "demo_QAMPER_IVIM.m", which is very useful. But as Git gets clogged up fast with files, we do not want nii files in Git. I see you have removed the test data (good!) but obviously the demo now fails.

Luckily, our Git comes with some test data that is stored on Zenodo: https://zenodo.org/records/10696605. This data is automatically downloaded whenever needed, as we coded the git such that it runs utilities/data_simulation/Download_data.py and stores it in "download\Data". I am now running your code with the abdomen.nii.gz data, and this works! However, it required me to (1) download the data manually and (2) make a mask.

So my questions are:
1: can we make sure the data is downloaded automatically whenever we need it for you demo (maybe this is not possible... Then you can refer to the location in the file)
2: is it required to add a mask.nii.gz or can it run without a mask/an automated mask?

@oliverchampion
Copy link
Collaborator

I think there are several files in here that are not needed, such as the .asv files. Could you remove the asv files, and add .asv to the .gitignore?

@locastre
Copy link
Author

locastre commented Sep 9, 2024

Apologies for the late responses on my part, I have not been receiving the notifications for this thread. I'll work on addressing the ASV and demo scripts now, sending update when they are complete.

@oliverchampion
Copy link
Collaborator

oliverchampion commented Oct 18, 2024

Apologies for the late responses on my part, I have not been receiving the notifications for this thread. I'll work on addressing the ASV and demo scripts now, sending update when they are complete.

@locastre, Any updates :)?

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