[FIX] Reduce user-defined mask when there is no good signal #172
Conversation
|
We're going to need to update |
Codecov Report
@@ Coverage Diff @@
## master #172 +/- ##
==========================================
+ Coverage 52.59% 52.71% +0.12%
==========================================
Files 32 32
Lines 1905 1912 +7
==========================================
+ Hits 1002 1008 +6
- Misses 903 904 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #172 +/- ##
==========================================
+ Coverage 52.59% 52.66% +0.06%
==========================================
Files 32 32
Lines 1905 1914 +9
==========================================
+ Hits 1002 1008 +6
- Misses 903 906 +3
Continue to review full report at Codecov.
|
| if np.any(masksum[mask] == 0): | ||
| n_bad_voxels = np.sum(masksum[mask] == 0) | ||
| LGR.warning('{0} voxels in user-defined mask do not have good ' | ||
| 'signal. Removing voxels from mask.'.format(n_bad_voxels)) |
There was a problem hiding this comment.
Could we add a "#TODO" about asking them to visually inspect the mask ? This is something we should include in the visual reports, eventually.
There was a problem hiding this comment.
I can do that, although we aren't actually writing out the masks at the moment. I was planning to incorporate writing out the masks and other intermediate outputs into verbose in a future PR, but can at least write out the masks with verbose in this one.
There was a problem hiding this comment.
Oh sorry I misinterpreted your comment. I'll add a comment to the code.
| @@ -133,8 +133,7 @@ def test_make_adaptive_mask(): | |||
| mask, masksum = utils.make_adaptive_mask(data, mask=pjoin(datadir, | |||
| 'mask.nii.gz'), | |||
| minimum=False, getsum=True) | |||
| assert np.allclose(mask, nib.load(pjoin(datadir, | |||
| 'mask.nii.gz')).get_data().flatten()) | |||
| assert np.allclose(mask, masksum.astype(bool)) | |||
There was a problem hiding this comment.
Our test is failing, here, since we're now hitting this "bad voxel" condition, I suppose ? This is probably actually fine for now since it covers more lines of code, but we could add a "#TODO" here about testing with a mask that does not include bad voxels.
There was a problem hiding this comment.
Yeah, a lot of voxels in the mask had bad data, so the mask file didn't match up with the adaptive mask. I'll add a TODO.
tedana/workflows/tedana.py
Outdated
| @@ -285,6 +285,9 @@ def tedana_workflow(data, tes, mask=None, mixm=None, ctab=None, manacc=None, | |||
| mask, masksum = utils.make_adaptive_mask(catd, mask=mask, | |||
| minimum=False, getsum=True) | |||
| LGR.debug('Retaining {}/{} samples'.format(mask.sum(), n_samp)) | |||
| if verbose: | |||
| io.filewrite(mask.astype(int), op.join(out_dir, 'mask.nii'), ref_img) | |||
There was a problem hiding this comment.
Is there a benefit in writing out the mask ? Either it's user-provided or effectively just the adaptive mask, no ?
There was a problem hiding this comment.
Just the regular mask? There's probably no reason to write it out. I'll drop that one.
|
This is great ! Thanks @tsalo ✨ Merging ! |
jbteves
added
breaking change
Closes #171.
Changes proposed in this pull request:
make_adaptive_mask), they will now be removed from the mask used intedana.