-
Notifications
You must be signed in to change notification settings - Fork 95
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
Concerns regarding TE-(in)dependence metric calculation #223
Comments
This is a great write-up ! I think it would be valuable to take some of these points to the next developers's call to discuss as a group. WDYT, @tsalo ? |
Absolutely. Sounds like a plan. |
There has been very little discussion on this since its posting, and this is a pretty big issue. I'm afraid that others (including myself) look at the size of the issue and immediately start having heart palpitations over how big it is. I would like to propose that we take all of the concerns above, including many which actually have a solution spelled out, and make them into their own issues. This is an excellent write-up, though, so maybe we should think about how to incorporate elements of it into documentation or comments in code. |
I can take certain concerns/problems and open them as separate issues, but some of them only make sense (at least to me) when considered within the overall metric calculation function. |
True. Perhaps this is a "hold out until the next hackathon" type of problem. WDYT? |
I've started opening specific issues and linking them here. I think this issue will still be useful, but we can focus discussion on those smaller issues. But I also agree that actually dealing with all of these issues will most likely have to wait until we have a hackathon. |
Sounds great to me! Thanks! |
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: ! |
Bot, you're right that this is a little stale, but please keep it open anyway. kthx |
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: ! |
I'm going to close this since there are multiple smaller issues open and this issue is stale. |
Summary
I would like to better document and understand how model metrics are computed. I have some concerns about some of the steps- especially how
WTS
is calculated and how regressions withget_coeffs
andcomputefeats2
are performed.Additional Detail
WTS
) withcomputefeats2
and normalized mixing matrix (when provided, otherwise regular mixing matrix).computefeats2
does not return un-normalized weights. It computes the weights using the data after it has been z-scored over time. It then caps the weights at +/- 0.999 and z-transforms them using Fisher's transform as if they were correlation coefficients.PROBLEM: I'm pretty sure that the normalized mixing matrix, which is only used with PCA, is actually z-scored along the wrong axis.FIXED WITH [FIX] Normalize PCA mixing matrix over time, not component #228tsoc_B
) withget_coeffs
and unnormalized mixing matrix.get_coeffs
and drop theadd_const
argument (per get_coeffs run on mean-centered data for no reason #179).get_coeffs
.tsoc_B
) divided by total variance from 3b.fitmodels_direct
do not seem to align well with the variance explained values we get directly from PCA, which makes me doubtfitmodels_direct
.WTS
) divided by total variance from 2b.X
) for S0 and T2* models.WTS
) over voxels.computefeats2
. Why would we do this again?full_sel
) is desired, calculate additional metrics.getfbounds
function and cluster size threshold based on number of voxels in mask.F_R2_clmaps
andF_S0_clmaps
arrays.countsigFR2
andcountsigFS0
arrays.tsoc_B
) using a cluster-defining threshold of the number of voxels in the mask minus the number of significant voxels from thecountsigFR2
/countsigFS0
variables and the same cluster-extent threshold as is applied to theF_R2_clmaps
/F_S0_clmaps
arrays.Br_R2_clmaps
/Br_S0_clmaps
(these arrays) as there are in the corresponding component maps fromF_R2_clmaps
/F_S0_clmaps
, but then after cluster-extent thresholding they would have fewer. Does the difference in number of significant voxels cause any problems?Br_R2_clmaps
andBr_S0_clmaps
should differ in terms of the number of voxels that survive thresholding because of the different cluster-defining thresholds determined byF_R2_clmaps
/F_S0_clmaps
, but the underlying data (tsoc_B
) is exactly the same.Next Steps
fitmodels_direct
look through the code and my description to see if I have anything wrong and to address any of the issues I've raised?The text was updated successfully, but these errors were encountered: