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

[WIP, FIX] remove bold_mask generation from t2s #1800

Closed
wants to merge 5 commits into from
Closed

Conversation

emdupre
Copy link
Collaborator

@emdupre emdupre commented Sep 30, 2019

Changes proposed in this pull request

Addresses #1786

Removes bold_mask generation from bold_t2s_wf in the case of multi-echo data.

Documentation that should be reviewed

N/A.

@emdupre
Copy link
Collaborator Author

emdupre commented Oct 1, 2019

This code is passing locally, and it seems like the errors here are in the cache:

FileNotFoundError: [Errno 2] No such file or directory: 'result_csf_msk.pklz'

or due to build time-out:

Too long with no output (exceeded 2h0m0s)

I'll try to clear the cache and re-run.

@effigies
Copy link
Member

effigies commented Oct 2, 2019

@emdupre I'm not sure this is a cache issue. Can you try updating the nipype requirement in setup.cfg to:

nipype @ git+https://github.com/effigies/nipype.git@e206f3d53198c683e959fe1e73426ce5cf362b93

@emdupre
Copy link
Collaborator Author

emdupre commented Oct 3, 2019

Thanks for the pin, @effigies ! That worked perfectly 👍

The brain masks here for ds210 don't look great, but they do locally for some data @Shotgunosine shared. Should I run (a non-downsampled) ds210 locally as well to confirm ?

@effigies
Copy link
Member

effigies commented Oct 3, 2019

@emdupre Yeah, I would go ahead and check on full-resolution (though you might want to truncate to ~40 TRs to save time).

@Shotgunosine
Copy link
Contributor

Yeah, those maps do look problematic, hopefully they'll look better on full resolution. It looks like it's mostly including skull as opposed to excluding brain, which is a less drastic error I think.

@effigies
Copy link
Member

effigies commented Oct 4, 2019

@emdupre I merged master, so you'll want to pull before doing anything else. Feel free to rebase, if you'd rather not have the pin and merge in the history,

@tsalo tsalo mentioned this pull request Apr 29, 2020
@emdupre
Copy link
Collaborator Author

emdupre commented Apr 29, 2020

Superceded by #2109

@emdupre emdupre closed this Apr 29, 2020
@effigies effigies deleted the fix/t2s-wf branch August 19, 2020 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants