Skip to content
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

Incorrect implementation and/or description of adaptive_mask #679

Closed
tsalo opened this issue Feb 11, 2021 · 4 comments · Fixed by #1061
Closed

Incorrect implementation and/or description of adaptive_mask #679

tsalo opened this issue Feb 11, 2021 · 4 comments · Fixed by #1061
Labels
bug issues describing a bug or error found in the project discussion issues that still need to be discussed

Comments

@tsalo
Copy link
Member

tsalo commented Feb 11, 2021

Summary

I was reviewing make_adaptive_mask() recently, and I noticed that the documentation and implementation don't line up. Specifically, the docstring implies that the adaptive_mask reflects the last echo with good signal, while the implementation indicates that it's the total number of echoes with good signal.

Additional Detail

Here's the part that sums good echoes instead of identifying the last good one:

tedana/tedana/utils.py

Lines 91 to 93 in 1a45f12

# determine samples where absolute value is greater than echo-specific thresholds
# and count # of echos that pass criterion
masksum = (np.abs(echo_means) > lthrs).sum(axis=-1)

When I directly checked which echoes had "bad data" in Logan's eight-echo data, according to the current implementation, the adaptive_mask generally had a value of 7 (indicating that the first 7 of 8 echoes are good), but the actual bad echoes were all either the first or second one!

If anyone has a good rationale for this behavior, please respond.

Next Steps

  1. Decide what we want from the adaptive mask:
    1. The first echo with bad data, minus one. I think this is the obvious interpretation of the adaptive mask, with the assumption (but not requirement) that echoes after that will also be bad. However, at least in some datasets, it looks like this will eliminate a lot of voxels.
    2. The total number of echoes with good data. This is the current implementation, but IMHO doesn't make sense given how we use the mask.
    3. Which specific echoes have good data. To do this, we would need to reorganize the adaptive mask from an (S,) array of integers into an (S x E) array of booleans. Our actual masking procedure would also need to be updated. This proposal also comes up in moving eimask to execute in make_adaptive_mask #491. Personally, I think this is the best option.
  2. Update either the documentation or implementation.
@tsalo tsalo added bug issues describing a bug or error found in the project discussion issues that still need to be discussed labels Feb 11, 2021
@tsalo
Copy link
Member Author

tsalo commented Feb 12, 2021

If we care about patterns of echoes, rather than the first X echoes, then we'll need to evaluate all subsets of echoes if we want to incorporate r-squared (i.e., #543), which is a lot to do. We'll need to figure out some shortcuts...

We'll also need to figure out the best way to incorporate our adaptive mask thresholds. For example, in a five echo dataset, if the fourth echo is the only good one, which two echoes do we use for t2smap?

@tsalo
Copy link
Member Author

tsalo commented Feb 19, 2021

A summary from today's call:

  1. Early echoes shouldn't be flagged by the adaptive mask if later echoes are good. This procedure is specifically aimed at identifying later echoes where signal drops off to zero. This means that, if the procedure is flagging early echoes, we should take a hard look at how we're deriving the adaptive mask.
    • This also means that my above comment about worrying about patterns of echoes is not as much of an issue. We ultimately should only care about the first X good echoes, rather than a potentially noncontiguous pattern of good echoes.
  2. I need to replicate my findings on patterns of echoes with some minimally preprocessed public data (see Add dataset fetching module #684).
  3. We can discuss improvements to the adaptive mask generation approach in this issue.

@tsalo
Copy link
Member Author

tsalo commented Feb 21, 2024

Improvements we can make:

  1. Define good echo value in adaptive mask based on last good echo for the voxel, rather than the sum of good echoes.
    • For example, if the pattern for a five-echo voxel is [Bad, Good, Good, Bad, Bad], we should set the adaptive mask value to 3 rather than 2, as the last good echo is the third one.
  2. Set a voxel's echo value to bad if the value isn't lower than the previous echo's value (Use overall signal decay in adaptive mask creation #312).
    • For example, if the values for a voxel are [10, 6, 5, 4, 4], then the result should be 4, since the last echo has a value that is equal to the second-to-last echo's value.
  3. Calculate R-squared (Supplement or replace adaptive mask with R-squared #543).

@tsalo
Copy link
Member Author

tsalo commented Mar 15, 2024

Continuing from #679 (comment) (specifically # 2), I checked the pattern of flagged echoes in a 5-echo run I have on hand. Specifically, I used the mask-limited version of the adaptive mask proposed in #1060.

Here's the number of voxels following each pattern (0 means a good echo, 1 means a bad echo):

{'00000': 2620,
 '00001': 289,
 '00010': 6,
 '00011': 92,
 '00100': 30,
 '00101': 22,
 '00110': 7,
 '00111': 124,
 '01000': 104,
 '01001': 16,
 '01010': 2,
 '01011': 9,
 '01100': 36,
 '01101': 17,
 '01110': 4,
 '01111': 143,
 '10000': 8087,
 '10001': 506,
 '10010': 23,
 '10011': 128,
 '10100': 109,
 '10101': 61,
 '10110': 21,
 '10111': 125,
 '11000': 9596,
 '11001': 1094,
 '11010': 101,
 '11011': 460,
 '11100': 10106,
 '11101': 2949,
 '11110': 5562,
 '11111': 175259}

There are definite jumps in frequency for sequential good echoes (e.g., first three echoes are good, last two are bad), such as '10000': 8087, '11000': 9596, '11100': 10106, '11110': 5562, '11111': 175259. However, there are also a lot of voxels that exhibit non-sequential patterns (e.g., '11101': 2949). I think this broadly replicates what I found in Logan's 8-echo dataset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues describing a bug or error found in the project discussion issues that still need to be discussed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant