Move component sign determination into a separate function

[tsalo]

Summary

Determination of the optimal signs for components and sorting components by Kappa are two steps performed within dependence_metrics. These two steps seem extraneous to the actual calculation of dependence metrics, and therefore I'd argue that they should be moved into their own functions.

Update(s)

Component sorting can't be separated into its own function with the way things are currently set up, but sign determination is very easy.

Additional Detail

Sign determination should be performed before any metrics are calculated, because, as mentioned in #318, signs of components matter for metric calculation. Moving this out of dependence_metrics would allow us to use the correctly signed components in any number of alternative metric functions.

On the other hand, component sorting can happen after metrics are calculated. I would argue that any metrics that rely on the order of the components should include their own internal sorting (e.g., as implemented in #295), so the order shouldn't matter. Instead, the order of the components should primarily matter for users, when they manually investigate the components (either in the component table or in the component figures). Component sorting can still happen before component selection if needed, but really it can happen any time before component maps, time series, or tables are written out.

Next Steps

1. Create new function for component sign determination that takes in the mixing matrix and either beta maps or the optimally combined data, and returns the updated mixing matrix.
2. Create new function for component sorting that takes in the mixing matrix and component table and returns the sorted versions.
   • This may not even require its own function. It could just go into the workflow function.
3. Integrate functions into workflow.

[eurunuela]

Sign determination should be performed before any metrics are calculated, because, as mentioned in #318, signs of components matter for metric calculation. Moving this out of dependence_metrics would allow us to use the correctly signed components in any number of alternative metric functions.

Does this mean components are not correctly signed?

Anyway, if I understood correctly, metrics are not being properly calculated, which is quite an issue I would say.

[tsalo]

In the current workflow, components are initially arbitrarily signed. Then, within dependence_metrics the optimal sign is determined and some are flipped. This sign-flipping step can be separated from metric calculation, in case we want to be able to swap out dependence_metrics for another metric calculation function in the future. This would probably be best given our long-term goal of making tedana a sandbox for many multi-echo denoising methods.

Regarding the accuracy of the signs, I do have concerns (as outlined in #316 and discussed a bit in #318), but I believe that those concerns are distinct from the refactor requested here. I also agree that signing of the components should impact metric calculation, but in the current code I don't think it does. What I said in the original post for this issue is a mistake. We treat positive and negative component weights equally, but I don't think we should (see #318).

[eurunuela]

Thank you for the clarification. I'm trying to catch up with the open issues and this one sounded odd when I read it.

Anyway, I do agree in that the code should be separated for an easier implementation of features in the future.

[tsalo]

After digging into this a bit more, I don't think we can separate out the component sorting, because dependence_metrics writes out a handful of files (if verbose), and the components in those files would be in a different order than the components in other files. That said, it should still be pretty easy to move the component signing into its own function, since all that needs is the mixing matrix and data, and all it returns is the updated mixing matrix. I'll update the title to reflect this.

[stale]

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