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

Parellelization #58

Open
araikes opened this issue Jun 12, 2020 · 10 comments
Open

Parellelization #58

araikes opened this issue Jun 12, 2020 · 10 comments

Comments

@araikes
Copy link

araikes commented Jun 12, 2020

Thanks to those who are updating this (@Shotgunosine, @PeerHerholz, and anyone I didn't notice).

Given that the openmp flag is being passed to recon-all should this also include -parallel (i.e.
https://github.com/BIDS-Apps/freesurfer/blob/d51c7e4505c15bd34b3a62be1cb233bdf36c18fb/run.py#L247

Thanks

@Shotgunosine
Copy link
Contributor

Hi @araikes thanks for the suggestion. If I understand their documentation right, that parallel option is for parallelization over subjects. Is that right?

@PeerHerholz
Copy link
Collaborator

Depending on the implementation and scope, I think we could also check out nipype's ReconAll implementation as we could nicely build upon the workflow architecture and already supported plugin options.

@PeerHerholz
Copy link
Collaborator

I just saw that the -parallel flag was removed in v6.0.1-4 (see corresponding PR here), apparently because of stability issues. However, I think this was related to OpenNeuro and since they don't support running of BIDS apps anymore, we could reintroduce it. However, the symlink problem is still a thing depending on the setup I guess. WDYT?

@araikes
Copy link
Author

araikes commented Jul 1, 2020

@Shotgunosine I'm fairly certain that the -parallel flag enables parallel processing of the hemispheres while also using the openmp parallelization. However, if the implementation is switched to nipype, I don't know if there would be noticeable performance gains for most.

@Shotgunosine
Copy link
Contributor

@PeerHerholz I think switching to a nipype backend might be a good thing to try, but would be fairly challenging. We'd need someone with time, skill, and interest to work on that problem. Now that I think about it, I may have had similar race conditions using the parallel flag on our HPC. Unless there is a strong demand for the feature, I'm not inclined to reintroduce it.

@Shotgunosine
Copy link
Contributor

@PeerHerholz If we were going to switch to a nipype backend, would we basically just be looking at creating a wrapper around https://github.com/nipreps/smriprep?

@PeerHerholz
Copy link
Collaborator

Hi gang,

sorry I didn't mean to re-route the discussion to a potential nipype implementation, just thought that wrt parallelization (among other things) it's an interesting opportunity. However, I agree with you @Shotgunosine: this would change the entire setup and implementation we have so far. smriprep is obviously great, but I rather thought of it as it's own thing that incorporates ReconAll but also does other things. Thus, I guess if we discuss this further the nipype ReconAll interface would be a good starting point. Of course: always happy to discuss this! That might be a nice hackathon project!?

@araikes could you maybe follow up on @Shotgunosine's point re running into problems on HPC? Did you ever noticed a comparable problems (using other ReconAll implementations than this BIDS app)?

@araikes
Copy link
Author

araikes commented Jul 1, 2020

@PeerHerholz and @Shotgunosine,

I can't say that I've noticed race conditions using either non-BIDS-app recon-all or this one (I forked the repository like a year ago and added the flag in locally). I'll do some more testing this/next week and see if it any issues crop up or if it seems to actually improve speed noticeably.

@PeerHerholz
Copy link
Collaborator

Thx @araikes, that sounds cool. The level of detail is highly appreciated!

@PeerHerholz
Copy link
Collaborator

Hi @araikes,

just wanted to ask if you have any updates here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants