Adding a degree of freedom (dof) flag (--n-independent-echos) to allow users to set the dof used in fstat calculations #1177

katielamar · 2025-02-25T00:12:10Z

Hi everyone,
I've updated the tedana code to address issue #1168. I've made this a draft PR since this is my first time contributing to Tedana and I'm a bit new to submitting pull requests. Let me know if you have any suggestions on how I can improve the documentation/code.

Closes #1168.

Summary of Updates:

User can now set the degree of freedom that will be used for the fstat calculation. More info: No BOLD components detected error #1168.
Tedana from CMD Line: Use --n-independent-echos to set the degrees of freedom when running tedana workflow from the command line
Tedana from Python: Set the variable echo_dof when calling the tedana_workflow function

tedana/metrics/dependence.py

handwerkerd

Besides the one ongoing discussion about a warning message, the rest of this looks good.

We should definitely include some tests. I can help with those, if useful.

adding tests. simplifying getfbounds

katielamar · 2025-03-08T18:19:07Z

@handwerkerd The tests/changes you made are in the latest commit. I also made a change to fix the fstat map test you added for echo_dof.

assert np.min(f_t2_maps_orig - f_t2_maps) >= 0
assert np.min(f_s0_maps_orig - f_s0_maps) >= 0

One of the asserts is still failing in fstat map test. Its caused by the following failing. Should this test be strictly greater than 0? I think maybe the f_max may be preventing that. Can you take a look?

assert np.min(f_t2_maps_orig[adaptive_mask == 5] - f_t2_maps[adaptive_mask == 5]) > 0
assert np.min(f_s0_maps_orig[adaptive_mask == 5] - f_s0_maps[adaptive_mask == 5]) > 0

tedana/tests/test_metrics.py

handwerkerd · 2025-03-11T02:08:02Z

Since the tests were passing on my local computer & failing here, I decided to push some commits to see if I could just solve it. I think the key problem was we have a maximum F value set to 500. That means, for a test >500, the F is a constant 500 and the difference between DOF==3 vs 5 is 0. Since the goal of the test was to make sure the change in DOF always changed the F stats, I needed to mask out the F==500 tests for both f_t2 and f_s0.

handwerkerd · 2025-03-11T02:11:03Z

@tsalo Do you want to take another look or is this good to merge?

tsalo

I have a few small requested changes. The main thing I'd like to see changed is to consistently refer to these numbers as either the number of independent echoes or the degrees of freedom. AFAIK the latter is one less than the former, but the terms are used interchangeably in this PR.

I'm also not clear on how the adaptive mask is used when echo_dof is defined. It's not used in the F-statistic estimation, but is it still used to calculate T2* maps and optimally combine data across echoes?

tsalo · 2025-03-11T15:28:46Z

tedana/metrics/collect.py

+        if echo_dof is None:
+            f_thresh, _, _ = getfbounds(len(tes))
+        else:
+            f_thresh, _, _ = getfbounds(echo_dof)


It would be simpler to just set echo_dof as len(tes) if it's not provided, at the top of this function.

However, I am concerned that setting the degrees of freedom based on the number of echoes is already a bug (see #811). It just doesn't account for the fact that the degrees of freedom varies across voxels due to the adaptive mask.

I agree on this change.
Addressing #811 might be useful, but seems beyond the scope of this PR. If we're clarifying which inputs are supposed to be DOF, this might be easier to address.

tsalo · 2025-03-11T15:31:16Z

tedana/selection/selection_nodes.py

+        # DOF is number of echoes if not otherwise specified
+        echo_dof = selector.cross_component_metrics_["n_echos"]


Just to clarify- DOF wouldn't be the number of echoes, right? It would be the number of echoes minus 1.

ug. I'm fairly sure in all calculations that use echo_dof, it's echo_dof - 1. The math is correct, but the terminology is wrong. This also brings up an issue for @katielamar in that, if echo_dof is the true DOF, should we run calculations with echo_dof and not echo_dof - 1?

I think a terminology-consistent solution should be to make echo_dof the DOF, when we calculate this based on the number of echoes, echo_dof = n_echoes - 1 and we'd then need to update all calculations that use these values to not subtract another -1.

You comment below of changing echo_dof to n_independent_sources is another solution that would not require changing formulas.

Hmm yeah I'm not sure. Im going to check with Tom on this and Ill get back to you.

tsalo · 2025-03-11T15:32:21Z

tedana/selection/tedpca.py

+    if echo_dof is None:
+        fmin, fmid, fmax = getfbounds(n_echos)
+    else:
+        fmin, fmid, fmax = getfbounds(echo_dof)


Suggested change

if echo_dof is None:

fmin, fmid, fmax = getfbounds(n_echos)

else:

fmin, fmid, fmax = getfbounds(echo_dof)

echo_dof = echo_dof or n_echos

fmin, fmid, fmax = getfbounds(n_echos)

tsalo · 2025-03-11T15:33:15Z

tedana/stats.py

+    f05 = stats.f.ppf(q=(1 - 0.05), dfn=1, dfd=(echo_dof - 1))
+    f025 = stats.f.ppf(q=(1 - 0.025), dfn=1, dfd=(echo_dof - 1))
+    f01 = stats.f.ppf(q=(1 - 0.01), dfn=1, dfd=(echo_dof - 1))


I think it's misleading to call the parameter dof and then subtract 1, unless I'm misunderstanding the math. We should change the function to use the degrees of freedom and pass in n_echos - 1 as needed.

Or just change the echo_dof variable to n_independent_sources/n_independent_echos.

tsalo · 2025-03-11T15:40:01Z

tedana/utils.py

+        # For EPTI sequences, the way we use adaptive mask thresholding fails
+        # because sequential echoes have overlapping information.


Suggested change

# For EPTI sequences, the way we use adaptive mask thresholding fails

# because sequential echoes have overlapping information.

# For EPTI sequences, the way we use adaptive mask thresholding fails

# because sequential echoes have overlapping information and

# there is no clear mapping between the independent sources and the echoes.

If we knew how much each independent source contributed to each echo we could figure this out.

tsalo · 2025-03-11T15:49:54Z

tedana/utils.py

+                f"{n_3dof_voxels} voxels ({np.round(perc_3dof_voxels, decimals=1)}%) have fewer "
+                f"than {np.round(threshold_3dof)} "
+                "good voxels. These voxels will be used in all analyses, "
+                "but might not include 3 independant echo measurements."


Suggested change

"but might not include 3 independant echo measurements."

"but might not include 3 independent echo measurements."

tsalo · 2025-03-11T15:50:05Z

tedana/utils.py

+                f"{n_dof_voxels} voxels ({np.round(perc_dof_voxels, decimals=1)}%) have fewer "
+                f"than {np.round(threshold_dof)} good voxels. "
+                f"The degrees of freedom for fits across echoes will remain {echo_dof} even if "
+                "there might be fewer independant echo measurements."


Suggested change

"there might be fewer independant echo measurements."

"there might be fewer independent echo measurements."

tsalo · 2025-03-11T15:50:22Z

tedana/utils.py

+                "good voxels. These voxels will be used in all analyses, "
+                "but might not include 3 independant echo measurements."
+            )
+        # There's a separate warning about DOF if it's possible there's a DOF reduction.


Suggested change

# There's a separate warning about DOF if it's possible there's a DOF reduction.

# There's a separate warning about DOF if it's possible there's a DOF reduction.

tsalo · 2025-03-11T15:50:43Z

tedana/utils.py

+        # For EPTI sequences, the way we use adaptive mask thresholding fails
+        # because sequential echoes have overlapping information.
+        # Since EPTI has less dropout, it is unclear how often this will cause issues.
+        # To track this, we are flagging voxels that might mark less independant signal.


Suggested change

# To track this, we are flagging voxels that might mark less independant signal.

# To track this, we are flagging voxels that might mark less independent signal.

tsalo · 2025-03-11T15:51:17Z

tedana/utils.py

+                f"than {np.round(threshold_3dof)} "
+                "good voxels. These voxels will be used in all analyses, "


Suggested change

f"than {np.round(threshold_3dof)} "

"good voxels. These voxels will be used in all analyses, "

f"than {np.round(threshold_3dof)} good voxels. "

"These voxels will be used in all analyses, "

katielamar added 7 commits February 23, 2025 08:47

DOF changes made

554faa9

passing style test

e940df1

lint and pytest passing

05acdc9

echo_dof update ready

2673ea2

Adding --n-independent-echos flag. See Issue ME-ICA#1168

bd63bf7

Rerun tests

eb83997

rerun tests 2

cfde9a4

tsalo reviewed Feb 25, 2025

View reviewed changes

tedana/metrics/dependence.py Show resolved Hide resolved

updating documentation

6551f05

katielamar marked this pull request as ready for review February 25, 2025 18:52

handwerkerd requested changes Feb 26, 2025

View reviewed changes

added adaptive mask warning

731737a

katielamar requested a review from handwerkerd February 27, 2025 17:56

katielamar and others added 7 commits March 3, 2025 09:09

updated getfbound calls to use echo_dof

7a5df43

adding tests. simplifying getfbounds

c209c6a

added more tests. fixed t2smap CLI. fixed adaptive mask warnings.

b57dbb7

fixed bug I created in calc_kappa_elbow and calc_rho_elbow

eb4691a

fixed comments

53c46d0

Merge pull request #1 from handwerkerd/EPTI

ab07715

adding tests. simplifying getfbounds

fixing echo_dof fstat map test. t One test still failing

deb431a

handwerkerd reviewed Mar 10, 2025

View reviewed changes

tedana/tests/test_metrics.py Outdated Show resolved Hide resolved

Update tedana/tests/test_metrics.py

a126d7f

handwerkerd reviewed Mar 10, 2025

View reviewed changes

tedana/tests/test_metrics.py Outdated Show resolved Hide resolved

Update tedana/tests/test_metrics.py

a5964cf

handwerkerd reviewed Mar 11, 2025

View reviewed changes

tedana/tests/test_metrics.py Outdated Show resolved Hide resolved

Update tedana/tests/test_metrics.py

6b846cf

handwerkerd approved these changes Mar 11, 2025

View reviewed changes

tsalo requested changes Mar 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a degree of freedom (dof) flag (--n-independent-echos) to allow users to set the dof used in fstat calculations #1177

Adding a degree of freedom (dof) flag (--n-independent-echos) to allow users to set the dof used in fstat calculations #1177

katielamar commented Feb 25, 2025 •

edited

Loading

handwerkerd left a comment

katielamar commented Mar 8, 2025

handwerkerd commented Mar 11, 2025

handwerkerd commented Mar 11, 2025

tsalo left a comment

tsalo Mar 11, 2025

handwerkerd Mar 11, 2025

tsalo Mar 11, 2025

handwerkerd Mar 11, 2025

handwerkerd Mar 11, 2025

katielamar Mar 12, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

tsalo Mar 11, 2025

		# DOF is number of echoes if not otherwise specified
		echo_dof = selector.cross_component_metrics_["n_echos"]

		# For EPTI sequences, the way we use adaptive mask thresholding fails
		# because sequential echoes have overlapping information.

	"but might not include 3 independant echo measurements."
	"but might not include 3 independent echo measurements."

	"there might be fewer independant echo measurements."
	"there might be fewer independent echo measurements."

	# There's a separate warning about DOF if it's possible there's a DOF reduction.

	# There's a separate warning about DOF if it's possible there's a DOF reduction.

	# To track this, we are flagging voxels that might mark less independant signal.
	# To track this, we are flagging voxels that might mark less independent signal.

		f"than {np.round(threshold_3dof)} "
		"good voxels. These voxels will be used in all analyses, "

Adding a degree of freedom (dof) flag (--n-independent-echos) to allow users to set the dof used in fstat calculations #1177

Are you sure you want to change the base?

Adding a degree of freedom (dof) flag (--n-independent-echos) to allow users to set the dof used in fstat calculations #1177

Conversation

katielamar commented Feb 25, 2025 • edited Loading

handwerkerd left a comment

Choose a reason for hiding this comment

katielamar commented Mar 8, 2025

handwerkerd commented Mar 11, 2025

handwerkerd commented Mar 11, 2025

tsalo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katielamar commented Feb 25, 2025 •

edited

Loading