Closed tsalo closed 2 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 89.74%. Comparing base (
41dafe5
) to head (b17fe1f
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I haven't had a chance to do any tedana stuff today, but I'll try to get to this early next week. One quick comment. Since it's also sort-of a bug to you want to address https://github.com/ME-ICA/tedana/issues/679#issuecomment-1956766801 here and "Define good echo value in adaptive mask based on last good echo for the voxel, rather than the sum of good echoes."
I think I'd like to address that in a second PR so we can really pin down the effect of each change.
Every time I look at the masking code, I notice something else that seems off. This time I think it's a function that is more flexible than documented (not an actual problem). I noticed that, for the masking code, reshape_niimg
would sometimes take as input a file name, sometimes a 3D image array, and sometimes a 1D image array. I thought this was wrong and would crash on some, but it seems to work for all those conditions.
If it's a file name, it loads the file and then reshapes and if it's already a loaded image, it just uses it. If it's already 1D, it squeezes the file and is still 1D.
Not sure whether we want to update that docstring in this PR, but I figured I'd mention.
@handwerkerd I've opened #1070 about the function.
I just checked how this affects results on one of my 3-echo datasets. I'm sharing the results here so that it's logged with this PR.
I ran 25 subjects with the current main
, this PR with masktype=["dropout"]
and this PR with masktype=["dropout", "decay"]
. I used a user-provided mask for each run. This CSV file has the results voxel_count_table.csv
main
PR dropout
and PR dropout+decay
. main
loses 0-5 voxels while this PR loses 2-147. As expected no extra voxels are lost for decay since the threshold is just the first echo.Here are the maps for sub-25 (yellow is 3 good echoes, orange is 2 good echoes, and red is 1 good echo)
Main:
PR with dropout:
PR with dropout+decay:
As you can see the loses seem mostly in areas where dropout is common and ventricles.
This is a bug fix and the new results are what we've indended to do for a while, so I'm fine with these changes, but I wanted to document this and we'll want to clearly highlight this change in our next release notes.
Also, this turned out to be slightly more annoying to do than I expected because t2smap
doesn't save the adaptive masked. I ended up pulling out the code used to create and save the adaptive mask from tedana. In case anyone else wants to play with this, here's my under-documented function:
import glob
import os
from nilearn.masking import compute_epi_mask
from tedana import (
__version__,
io,
utils,
)
def running_adaptive_mask(data, n_echos, mask_file, masktype, out_dir, prefix):
""" Pulling out the code from tedana.py to run and save the adaptive mask """
catd, ref_img = io.load_data(data, n_echos=n_echos)
io_generator = io.OutputGenerator(
ref_img,
out_dir=out_dir,
prefix=prefix,
config="auto",
overwrite=True
)
first_echo_img = io.new_nii_like(io_generator.reference_img, catd[:, 0, :])
mask = utils.reshape_niimg(mask_file)
# Replace this line with the one above if you want to use the auto-generated mask from within tedana
# mask = compute_epi_mask(first_echo_img)
mask_denoise, masksum_denoise = utils.make_adaptive_mask(
catd,
mask=mask,
threshold=1,
methods=masktype,
)
io_generator.save_file(masksum_denoise, "adaptive mask img")
Closes #1059. I built this off of #1057, so it shares many changes.
Changes proposed in this pull request:
mask
a required argument formake_adaptive_mask
.compute_epi_mask
to calculate the mask in the t2smap workflow if one isn't provided.