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

Don't interpolate volumes at beginning/end of run #950

Merged
merged 23 commits into from
Oct 11, 2023

Conversation

tsalo
Copy link
Member

@tsalo tsalo commented Sep 29, 2023

Closes #949.

Changes proposed in this pull request

  • After interpolation, but before bandpass filtering, replace any interpolated volumes at the beginning or end of the run with the closest non-outlier volume's data. This should act much like the constant buffer we use for the bandpass filter.

Documentation that should be reviewed

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

All modified lines are covered by tests ✅

Files Coverage Δ
xcp_d/utils/utils.py 100.00% <100.00%> (ø)

📢 Thoughts on this report? Let us know!.

@tsalo tsalo added bug Issues noting problems and PRs fixing those problems. breaking-change PRs that change results or interfaces. labels Oct 9, 2023
@tsalo tsalo added this to the manuscript milestone Oct 10, 2023
@tsalo tsalo requested a review from mattcieslak October 10, 2023 14:26
@tsalo
Copy link
Member Author

tsalo commented Oct 10, 2023

@mattcieslak would you mind looking over the core change?

@tsalo tsalo removed the breaking-change PRs that change results or interfaces. label Oct 10, 2023

To address this, XCP-D now replaces interpolated volumes at the edges of the run with the
closest non-outlier volume's data, as of 0.5.1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an appropriate level of concern/warning

"sub-1648798153_ses-PNC1_task-rest_acq-singleband_desc-confounds_timeseries.tsv",
)
motion_df = pd.read_table(motion_file)
motion_df.loc[56:, "trans_x"] = np.arange(1, 5) * 20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great idea

# Create an updated censoring file with outliers at first and last two volumes
censoring_df = confounds_df[["framewise_displacement"]]
censoring_df["framewise_displacement"] = False
censoring_df.iloc[:2]["framewise_displacement"] = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you pick 2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was just arbitrary. It could have been any number >= 1.

xcp_d/utils/utils.py Outdated Show resolved Hide resolved
if outlier_idx:
gaps = [[s, e] for s, e in zip(outlier_idx, outlier_idx[1:]) if s + 1 < e]
edges = iter(outlier_idx[:1] + sum(gaps, []) + outlier_idx[-1:])
consecutive_outliers_idx = list(zip(edges, edges))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is hard for me to follow intuitively, but I tested it a bunch and it seems to work fine.

] = interpolated_unfiltered_bold[first_outliers[1] + 1, :]

# Replace outliers at end of run
if last_outliers[1] == n_volumes - 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem only arises if the very last or very first points are censored?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems like cubic spline interpolation works well as long as there's some data before and after the points being interpolated, so the problem comes from interpolating with point only on one side.

@tsalo tsalo self-assigned this Oct 11, 2023
@tsalo tsalo merged commit e0f0848 into PennLINC:main Oct 11, 2023
4 checks passed
@tsalo tsalo deleted the stop-interpolating-ends branch October 11, 2023 14:42
tsalo added a commit to tsalo/xcp_d that referenced this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues noting problems and PRs fixing those problems.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cubic spline interpolation at beginning/end of scan + bandpass filtering causes artifacts
2 participants