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

Drop PAID support from tedana workflow #214

Closed
tsalo opened this issue Feb 9, 2019 · 1 comment
Closed

Drop PAID support from tedana workflow #214

tsalo opened this issue Feb 9, 2019 · 1 comment
Labels
discussion issues that still need to be discussed priority: low issues that are not urgent

Comments

@tsalo
Copy link
Member

tsalo commented Feb 9, 2019

Summary

In #210, I assumed that we wouldn't support the PAID combination method in the denoising workflow because I believe it is only valid on smoothed data and we don't want to perform denoising on smoothed or warped data. However, we do currently support PAID, which is why I'd like to discuss the possibility of removing that support.

Additional Detail

PAID assumes that "noise is isotropic and homogeneous throughout the image", which is only the case after smoothing. Since we don't want to perform multi-echo denoising (i.e., the tedana workflow) on smoothed data, I don't think that PAID should be an option for tedana_workflow, even though we should retain it for t2smap_workflow.

@tsalo tsalo added discussion issues that still need to be discussed priority: low issues that are not urgent labels Feb 9, 2019
@emdupre
Copy link
Member

emdupre commented Feb 11, 2019

To expand on this a little more: The idea, then, would be that T2* map generation could be used in any workflow, and in those with smoothed input data, the PAID method would be reasonable to use. But, because the tedana_workflow includes decomposition and denoising, we want to make sure that's performed on unsmoothed data. Is that right, @tsalo ?

If so, I think @handwerkerd and @javiergcas will have the most context for this, but I'm inclined to agree with your suggestion.

To confirm, then, concrete steps would be to remove the ste flag from here (and subsequent workflow steps) but leave it in place here ? We would also need to update the documentation for that option in t2smap_workflow to explain its relative utility (and provide a reference, as we do for t2s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion issues that still need to be discussed priority: low issues that are not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants