[FIX] Improve bounding box computation from masks #304
Conversation
| int((mask_data.shape[1] * mask_data.shape[2]) * 0.2), # sagittal | ||
| int((mask_data.shape[0] * mask_data.shape[2]) * 0.0), # coronal | ||
| int((mask_data.shape[0] * mask_data.shape[1]) * 0.3), # axial | ||
| ] |
There was a problem hiding this comment.
This is opaque to me. ijk_th reads like threshold, but I don't see how this should produce a threshold.
|
This patch still does not address the real underlying problem (it would involve resampling the mask into an affine that is aligned with the canonical axes) but it does the job. |
|
@effigies, please let me know if I managed to clarify the idea with the comments. If so, feel free to merge. |
| ras_coords.append(apply_affine(mask_nii.affine, cross).tolist()) | ||
|
|
||
| ras_coords.append(apply_affine( | ||
| mask_nii.affine, cross).tolist()) |
There was a problem hiding this comment.
Is it possible that the problem is actually just that we don't make sure these are integers, being sure to round up at the bottom and down at the top?
There was a problem hiding this comment.
nilearn takes physical coordinates, so I doubt this is the problem.
There was a problem hiding this comment.
Right, but if we round the physical coordinates to outside the mask, then we might get undefined behavior. We certainly get something.
There was a problem hiding this comment.
A warning saying that no contours were found was thrown. Nilearn used to not plot anything in that case, but after some release the weird contours would show up. The thresholds solution should take care of most of the cases where this rounding happened, as we don't compute the bounding box of the mask, but the bounding box of the mask with a decent amount of masked pixels instead. The tests were failing with very small masks, but 55aa451 should fix that falling back to the old calculation (with your suggestion in #304 (comment)).
That said, the issue I'm afraid, is that nilearn rotates the images (and the masks) to align with canonical axes in visualization. As the brain is not perfectly spherical, these rotations might force the coordinates outside the brain if you calculate them exactly at the brain surface (as we were doing) for very tilted brains.
I think that this implementation will basically satisfy 99.9% of the cases and the remaining 0.1% will have the weird contours - which are confusing but not harmful.
| mask_data = mask_nii.get_data() | ||
| B = np.argwhere(mask_data > 0) | ||
| start_coords = B.min(0) | ||
| stop_coords = B.max(0) + 1 |
There was a problem hiding this comment.
Or maybe this should have just been stop_coords = B.max(0)?
There was a problem hiding this comment.
This indeed seemingly fixed the issue (that was the first thing I tried). However, being at it, I wanted to improve the visualization because such a liberal calculation usually gave almost empty views of the axial plane in many images.
Co-Authored-By: oesteban <[email protected]>
|
Reports are looking good - thumbs up? |
While nilearn/nilearn#25 is considered, this PR advances the generation of mosaics after aligning the affine matrix of the data with the cardinal axes. This should overcome the glitches when nilearn is requested to plot contours and the mask for the ROI has voxels that only partially intersect with the visualization plane. Resolves: nipreps#542. References: nipreps#281, nipreps#304. Depends: nipreps#543.
While nilearn/nilearn#25 is considered, this PR advances the generation of mosaics after aligning the affine matrix of the data with the cardinal axes. This should overcome the glitches when nilearn is requested to plot contours and the mask for the ROI has voxels that only partially intersect with the visualization plane. Resolves: nipreps#542. References: nipreps#281, nipreps#304. Depends: nipreps#543.
While nilearn/nilearn#25 is considered, this PR advances the generation of mosaics after aligning the affine matrix of the data with the cardinal axes. This should overcome the glitches when nilearn is requested to plot contours and the mask for the ROI has voxels that only partially intersect with the visualization plane. Resolves: nipreps#542. References: nipreps#281, nipreps#304. Depends: nipreps#543.
While nilearn/nilearn#25 is considered, this PR advances the generation of mosaics after aligning the affine matrix of the data with the cardinal axes. This should overcome the glitches when nilearn is requested to plot contours and the mask for the ROI has voxels that only partially intersect with the visualization plane. Resolves: nipreps#542. References: nipreps#281, nipreps#304. Depends: nipreps#543.
Before (fieldmap of ds054 on fmriprep's reports at Circle):
After: