moving eimask to execute in make_adaptive_mask

[handwerkerd]

Closes #490

This addresses an issue with a secondary mask is applied to the data during PCA decomposition and the mask volume isn't updated so the program later crashes.

Changes proposed in this pull request:

• Moves eimask function from decomposition._utils to utils.py and executes that function within make_adaptive_mask

One general thing to note is that mask_adaptive_mask is currently a few fairly arbitrary thresholds. This will add another arbitrary threshold. Both these arbitrary thresholds are already in the code and this properly places them in the same place so it's a net positive in code clarity.

[handwerkerd]

I think I've made the necessary edits to the code, but I haven't had a chance to test this on my data yet.

[tsalo]

I also tinkered with a solution on my fork so I was able to identify a couple of issues that need to be resolved.

eimask returns an SxE boolean array, where bad voxel-echo combinations are False and good ones are True. This is different to make_adaptive_mask, which identifies the number of echoes with good data for each voxel. So for one voxel and five echoes, we might have the following:

• eimask: [True, False, False, True, True]
• adaptive_mask: 3
  • equivalent to [True, True, True, False, False]

Any code throughout the package that uses the adaptive mask just grabs the first X values from the data based on the number in the mask. If we use eimask as-is, we'll need to adapt our mask arrays and our related code to support selecting non-consecutive subsets of echoes.

I see two options there:

• Adapt eimask to identify the number of consecutive non-outlier echoes starting with the first one, instead of identifying each non-outlier echo independently.
  • Also make sure it returns an (S,) array of integers instead of an SxE array of bools.
• Adapt make_adaptive_mask and all other code to support voxel-specific combinations of "good" echoes. This would mostly impact the T2*/S0 estimation, but if we merge [ENH] Only use good echoes for metric calculation/optimal combination #358 (which we said we'd do at some point), it would impact metric calculation as well.

[handwerkerd]

In the current version of the code eimask is actually dysfunctional because it is supposed to have multi-echo data as an input, but we were inputting single echo data. My sense is that a conservative improvement is just to mask out any voxel that has bad data in any echo. The threshold in eimask is really conservative anyway (>5 times or less than 1/1000 of the 98th percentile value) so it's not like it's masking specific voxel/echoes for a subtle model-misfit. This is the approach I'm taking & will try to share tomorrow.

On a bigger picture, going down this rabbit hole made be realize how arbitrary all these masking thresholds are. I want something functional in the short-term, but I'd also need to think more about how this will interact with #358

[handwerkerd]

#513 notes another code section where data-less voxels within a mask causes problems. It would be good for this masking issue to be addressed in this PR on a more systematic level.

[handwerkerd]

As part of this PR, change masksum to adaptive_mask within masking-specific functions. See #448 and #358 (comment)

[tsalo]

@handwerkerd Would you be willing to change the permissions on the files in your repo from 100755 to 100644 to make it easier to compare changes?

[tsalo]

The code says "all" but the comment says "any". I noticed in the conversation that you were leaning toward any. Is that still accurate?

[handwerkerd]

Permissions should now be changed. FWIW, this is still a work in progress. My hope is to get #358 merged and then get this working on that code.

[stale]

This issue has been automatically marked as stale because it has not had any activity in 90 days. It will be closed in 600 days if no further activity occurs. Thank you for your contributions to tedana:tada: !

[handwerkerd]

@tsalo I'm fairly sure this has been resolved with #555. If you agree, you can close this.