-
Notifications
You must be signed in to change notification settings - Fork 173
Resection identification in Brainstorm #848
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
base: master
Are you sure you want to change the base?
Resection identification in Brainstorm #848
Conversation
IFF exactly two MRIs are selected. 1st is the pre-op MRI 2nd is the post-op MRI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks fine, there are some details to address:
-
Why is it called labeling? There is no labeling, but masking, or resection identification.
-
Is there a plotting in calling
resection_labeling
?
I got the messages that some plotting was happening but the figures did not show up. If there is plotting, can we move it to the process code so it plots Brainstorm figures?
EDIT (Add the message showed in the Matlab command window about plotting)
epoch_loss: �[94m -0.0753 �[0m for epoch: �[92m 2999/3000 �[0m
nilearn/plotting/find_cuts.py:154: UserWarning: Could not determine cut coords: All voxels were masked by the thresholding. Returning the center of mass instead.
warnings.warn(
Plotted preop MRI with resection mask for preop.nii.gz
Plotted postop MRI with resection mask for postop.nii.gz
- See the other individual comments
bst_report('Error', sProcess, [], ['Subject "' SubjectName '" is using the default anatomy (read-only).']); | ||
return | ||
end | ||
% Proceed importing the MRI files only if no anatomy defined for subject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want this limitation?
A way to give the user the option to do the same thing with GUI and with Processes would be to:
-
Modify
process_import_mri
to add an optionisAutoAdjust
(I'll handle this inmaster
) -
Then the pipeline would look:
Process IMPORT MriFile1 as MRI1 Process IMPORT MriFile1 as MRI2 Process RESECT(MRI1, MR2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your suggestion sounds much better. Definitely, now it's better to not have this limitation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The process process_import_mri()
handles now CT and PET volumes. Check b4b2da3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the user can give either the FileName
or the Comment
of the PostOp MRI.
And the resection identification will happen against the Default MRI for that Subject
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the user can give either the FileName or the Comment of the PostOp MRI.
And the resection identification will happen against the Default MRI for that Subject
From the process_resection_identification
, instead of asking for both the pre-op and post-op (as it is now) we just ask for post-op MRI (image below) as it is expected that the pre-MRI is already imported/set as the default MRI ?
If pre-MRI is not present then we give an appropriate message to user that the pre-MRI needs to be imported first. Does this sound good ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if PostOp MRI is already imported?
(This is the case as calling from the db tree)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, like we have the dropdown list for the Subject in the image above, we can have a dropdown list for choosing the post-op MRI from all the non-default MRI volumes
EDIT: How about just defining a text field for post-op MRI name in the process tab, where user can just type in the name of the postop file from the db tree ? Check e2f3e4d.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: How about just defining a text field for post-op MRI name in the process tab, where user can just type in the name of the postop file from the db tree ?
This was suggested in #848 (comment)
toolbox/core/bst_plugin.m
Outdated
switch(OsType) | ||
case 'linux64' | ||
PlugDesc(end).URLzip = 'https://neuroimage.usc.edu/bst/getupdate.php?d=bst_resection_labeling_linux.zip'; | ||
PlugDesc(end).TestFile = 'resection_labeling'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add execute permission to resection_labeling
(linux64
and mac64*
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should have it directly embedded in the binary ? Let me check this.
EDIT: Handled in the binary. Associated PR: ajoshiusc/auto_resection_mask#4
I just followed the naming as per the tutorial https://neuroimage.usc.edu/brainstorm/Tutorials/SegBrainSuite#Resection_labeling but agree
There is no plotting actually that needs to be displayed rather the plotting happens to generate two image files (below) for sanity check on the resection mask generation. Don't think we need to have them as Brainstorm figures as once the resection mask is imported we can overlay it on the preop MRI and check it. |
This custom directory is the path where the binary packages will be extracted. If not specified, it uses the system's temporary directory as the default.
- If it is an empty subject, import the pre-op, do MNI normalization, and then import the post-op. - If already anatomy present in the subject, then just import them as two additional MRI files to the subject. Since, `isAutoadjust` is set to 1, so they will reslice/resample based on the default MRI)
- The preop and postop MRIs need to be imported first to a subject before running the process. - The pre-op MRI is always the default MRI so no need to explitly ask for it in the process tab. - The post-op MRI could be any of the non-default MRI. To keep it simple, in the process tab user can type in the name of it as in the db tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Few extra comments:
-
The permissions to be executable does not seem to be properly set, at least in Linux. Test: download and unzip the
.zip
file -
There is still the legend about plotting, that one is confusing as the generated images are not shown, not even saved. Can you remove the generation of those images from the binaries?
nilearn/plotting/find_cuts.py:154: UserWarning: Could not determine cut coords: All voxels were masked by the thresholding. Returning the center of mass instead.
warnings.warn(
Plotted preop MRI with resection mask for preop.nii
Plotted postop MRI with resection mask for postop.nii
RESEC_ID > Computation completed in: 1256.6037s
- About the menu entry.
Does it fit better under MRI segmentation?
bst_report('Error', sProcess, [], ['Subject "' SubjectName '" is using the default anatomy (read-only).']); | ||
return | ||
end | ||
% Proceed importing the MRI files only if no anatomy defined for subject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: How about just defining a text field for post-op MRI name in the process tab, where user can just type in the name of the postop file from the db tree ?
This was suggested in #848 (comment)
The permissions are set now. I set them directly in the server.
Updated the binaries to remove the generation of the images.
Definitely a better fit. Check 98d7f19 |
1. Remove repeated check for MRIs to be from the same subject and not default anatomy from `Compute` because the checks are already done when calling from GUI. 2. Make sure the fiducials in pre-op MRI are set as the surface (from the resection masks) needs the SCS coordinates. 3. Add `ComputeInteractive` function.
This PR integrates the resection-identification from https://github.com/ajoshiusc/auto_resection_mask/tree/brainstorm-plugin as a Brainstorm plugin. Given a pre and post op MRI, this generates two resection masks:
(Discussed this with @ajoshiusc and this is the most relevant one that is used in the community, so I am just importing that).EDIT: As discussed on Slack, we import both the masks (as atlases) and the post2preop coregistered MRI from the plugin outputs.
EDIT2: It is also relevant that we generate the
resection volume
from the masks.Test data: Example data from the repo above.
select both of them,right-click on the postop and start the resection identification as seen below.Tutorial (WIP): https://neuroimage.usc.edu/brainstorm/Tutorials/ResectionIdentification