-
Notifications
You must be signed in to change notification settings - Fork 272
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
Extend the checking of the subcortical segmentation #165
Extend the checking of the subcortical segmentation #165
Conversation
…ely for each structure, and include additional info regarding overlap with the Atlas
This may be a bit of information overload. The original purpose of the feature was to assess overlap between the brain mask and the atlas ROIs, not the individual structure ROIs and the atlas ROIs, which are expected to have numerous mismatches in all subjects. It is very surprising that the first column is non-zero for more than the brain stem and possibly the cerebellum if this is the dilation subject. How is this occurring? |
So, this quantifies the mismatch the individual structure ROIs and the Atlas ROIs, which IMO is the more relevant info anyway, since it provides info on the extent to which the CIFTI voxels are actually informed by "real" underlying data, rather than simply filled in via the dilation process during CIFTI creation. The first column is non-zero for more than just the brain stem and cerebellum because other structures (e.g., hipp, amyg) also form the border with the brain mask. |
I guess I don't see how this additional info would be useful for evaluating data usability. Do you envision another use for it? |
Sure. One could set a threshold of the minimum amount of overlap with the Atlas that one is willing to accept to include that structure from that subject as part of an analysis. Having this available gives us a mechanism to get a handle on how pervasive of an issue this is. |
I am not following. As I have said, it is EXPECTED that there will not be perfect overlap between the atlas and individual ROIs. This is because although nonlinear volume registration does a decent job with subcortical structures, it does not do a perfect job and so there will be some differences in overlap. The purpose of the CIFTI approach to subcortical structures is to use the data from the individual's ROI to populate the data from the atlas ROI, rather than tolerating residual partial voluming of structures after a volume-based alignment. Assuming proper FOV coverage of the structures (and no artifacts like that brain stem case), the subject is usable even if there are many non-zero values in your table. It is not clear to me how the changes have enabled us to better discriminate usable cases and those with FOV issues or major artifacts. The fact that some atlas subcortical structures end up near the edge of the brain mask without major artifacts or FOV issues actually means that the extant table (and feature) is less useful than I thought it would be. Small differences between the brain mask and atlas ROIs are also expected if the atlas ROIs border the brain mask edge in some places. I guess what is really needed for determining data usability is something that detects large artifacts or FOV mismatches that occur over multiple contiguous slices. Thus, there needs to be some mm threshold built into these measures to ensure that small, expected variances are not being counted. Perhaps if you dilated the brain mask 2x the voxel size and measured atlas ROI voxels outside of that, that would be a useful measure. |
I would consider reordering the columns, having the number that is expected to be big when things are good buried in the middle is confusing. Maybe overlap, brainmask, missing from atlas, discarded from individual? |
Can we please focus on the accuracy of the code? The changes are simply quantifying the degree of overlap, so that we have a precise definition of it. I never said that the overlap was expected to be perfect, but I think we can agree that it also shouldn't be "awful". Currently, we have no way to quantify this. My additions provide it. |
@glasserm I think part of what Mike wants is to quantify how much of the data in the atlas cifti space is just dilation. A few voxels isn't a problem, yes, but until now we have basically been hiding this fact from the user entirely. |
It isn't clear why that needs to be a part of the pipelines versus some script Mike runs on some data to test things (I write such scripts all the time and only rarely do they become pipeline additions). I did think the original purpose of the code was sensible--to identify unusable subjects with major issues, but I don't think the implementation currently will do a good job at that. I suggested above an approach to improve upon this. |
We haven't yet decided what the threshold for unusable for this issue is, even in our own data. Dilating would only hide some of the non-overlap, making the measure less accurate and sensitive. Different users may want a different threshold for usability, or may even want to try it as a confound regressor. "It's not zero" isn't the only way to handle a measurement. |
The code above will not help in making a threshold determination either. I don't think the overall set of ideas is mature enough to be included in the HCP Pipelines at this time, as it does not serve the originally intended purpose (flag bad subjects) and the purpose it appears to be attempting to serve is really better done with a separate experimental script. Alternatively, a 2 voxels away threshold is reasonable and I think would be useful for the brain mask, if there was a desire to implement something now. I am less convinced of the utility of the individual versus atlas ROI analysis, but I suppose a 2 voxels away threshold could be used there. |
I believe that the current file provides all the information necessary to evaluate the overlap thoroughly. It also appropriately exposes the overlap to users that may care about this issue. |
It doesn't say how far the voxels are away, and that is CRITICAL to interpreting the information. |
I can add code that additionally adds I don't see value in a 2x dilation on the brain mask. In fact, once I got into code, and thought about it more, I don't see much value in the |
If we have 2x dilation for both the ROIs and the brain mask, I guess the code is okay for inclusion, as it at least could be used for the original purpose. |
What is the purpose of comparing to the brain mask at all? It serves no purpose. |
I'm about ready to test a revision. Do I have your blessing to drop the brainmask comparisons? If not, please provide a rationale as to what purpose they serve. |
The brainmask comparisons are the only numbers I would look at from your code and would address whether a subject has issues or not regarding usability. I don't intend to accept this pull request without them. I have explained their purpose repeatedly above. |
No you haven't. Not in the face of my point that "Given the parcel-constrained nature by which the CIFTI is created, it is irrelevant if the Atlas ROI voxels have spatial coverage by subject-specific voxels labelled with anything other than their specific associated Atlas structure". If half of the Atlas ROI for hippocampus is "covered" by voxels labelled as amygdala in the subject, those are dilation voxels for the purpose of setting the grayordinate values. |
The brainmask flags overall artifact or FOV issues that would warrant closer inspection. Segmentation or registration variances are a different class of issues that will likely be much more common and much less concerning. |
It baffles me how 50% of a structure being based on dilated values is "less concerning", but in the interest of making everyone satisfied, I will dilate the brainmask and include your additional requested variable. We can each then focus on what we think is most relevant. |
…so as to discount for voxels near the borders
Updated output. First column of numbers is the one that you requested.
|
Can "nROIOverlapAtlas" be changed to be last, as it looks like the only number where larger is better? I find it odd that most of the structures have similar nMissingROIFromAtlas to their overlap (always larger in this output), is this a particularly bad (or small) subject, were the atlas ROIs made using union rather than >50% of subjects, or something else (bug in the code)? |
I did it in that order because it seemed more logical to cover all the variables related to missing or overlap first. The structurals in this test subject weren't so great. |
Do these data exist somewhere I could look at them? |
It does seem in this subject that a lot of the anterior temporal lobes was missed by the FreeSurfer segmentation, though apparently in this run the brainstem was less of an issue. The dilated brain mask might be useful to keep around for any follow up checking of things, rather than having to infer what it would have looked like from the non-dilated mask. |
|
||
# Repeat with brain mask dilated by 2 x GrayordinatesResolution | ||
# Use -volume-dilate rather than 'fslmaths -dilF -dilF', for explicit control over the dilation distance | ||
dilDist=$(echo "2 * $GrayordinatesResolution" | bc -l) |
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.
Thinking about this, the expected nonoverlap doesn't really seem dependent on the voxel size, it depends more on how well the structurals are registered and segmented. If someone were to make a 4mm grayordinates space (because they hate being able to see any detail, presumably), this would dilate 8mm, while if resolution improves to 1.2mm, this would only dilate by 2.4mm.
Of course, in nonhuman subjects, the dilation distance would need to be scaled by brain size, and using the voxel resolution sort of does that.
Another option would be to actually measure the distance of all the non-overlap voxels from the ROI, and average them, but this would need a new wb_command implemented.
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.
I think it is okay because we are talking about specific standard grayordinates spaces.
@coalsont Are you still reviewing? |
I don't have any more comments on the current PR code. |
So, is this ready to merge? |
OK |
With these changes, it will now report separately for each structure, and include additional info regarding overlap with the Atlas ROIs.
Example output file (from the same subject that prompted us to add this report in the first place):
Note that the sum of
nROIOverlapAtlas
andnROIOutsideAtlas
is the number of voxels in the subject-specific ROI. And the sum ofnMissingROIFromAtlas
andnROIOverlapAtlas
is the number of voxels in the Atlas ROIs.Having this info readily available will be valuable I think. For example, in the example above, we can quickly discern that a number of structures had rather poor overlap with the Atlas ROIs (e.g.,
nMissingROIFromAtlas
>nROIOverlapAtlas
), such as PUTAMEN_LEFT, both AMYGDALA, and both ACCUMBENS.