bids-apps / freesurfer

BIDS app wrapping recon-all from FreeSurfer
Apache License 2.0
40 stars 35 forks source link

Freesurfer 7 version? #57

Closed eduardklap closed 1 year ago

eduardklap commented 4 years ago

Hi, thanks for making this bids tool! I was wondering whether you were planning to implement the new FreeSurfer 7.1 (https://surfer.nmr.mgh.harvard.edu/fswiki/rel7downloads) in the BIDS-app? Would be great! If I could be of any help, let me know. Could be something for the OHBM Hackathon next week, which I will be attending :)

Best, Eduard

PeerHerholz commented 4 years ago

Ahoi hoi @eduardklap,

sorry for the late reply. I think it should definitely be supported, but a complete switch from version 6 to 7 should happen at some later point in time. However, this brings up a very interesting topic: should we support/allow multiple versions simultaneously? I think at least 6 and 7 should be supported for continuity reasons. I guess we could work with version specific tags as it's already done on dockerhub. We could definitely start working on and testing version 7 in a dedicated branch.

eduardklap commented 4 years ago

Hi @PeerHerholz. Yes, would be great to have version 7 alongside version 6. I am happy to help out with testing stuff. Have experience with running but not building docker containers, but willing to learn something new :)

PeerHerholz commented 4 years ago

Hi @eduardklap,

cool, thanks for your interest in helping out! No worries re containers, we'll go on this exiting journey together. How about you fork the repo (if you haven't done it already), create a new branch and we go from there?

Cheers and thx again, Peer

Shotgunosine commented 4 years ago

I guess the ideal solution for this would involve a new release of neurodocker, but in the mean time, I've got a dockerfile that seems to make a freesurfer 7 version of the bids app here: https://github.com/Shotgunosine/freesurfer/blob/fs_7/Dockerfile_fs7

AndysWorth commented 3 years ago

I'm hoping to see an official BIDS App version of Freesurfer 7. What can I do to help?

Is this effort waiting for a new bug-fix release from the LCN/Martinos Center? I know there are at least two issues with the 7.1.1 release where there are fixes but I see no timeline for a 7.1.2 release. @Shotgunosine, a couple small changes to your fork/branch would fix those issues. Should I work on a PR on your branch or should that be merged into the BIDS-Apps repo first?

Shotgunosine commented 3 years ago

@AndysWorth, I'd be happy to accept a PR to my branch. @PeerHerholz what do you think about going ahead with my solution?

PeerHerholz commented 3 years ago

@Shotgunosine Yeah, I think the parallel is the way to go. For the foreseeable future we should support both 6 and 7. Having dedicated branches for each version should also allow parallel automated builds and tests. If @AndysWorth pushes the required fixes to your branch we could merge things at the higher level. WDYT?

Shotgunosine commented 3 years ago

sounds good to me

On Tue, Oct 20, 2020 at 11:03 AM Peer Herholz notifications@github.com wrote:

@Shotgunosine https://github.com/Shotgunosine Yeah, I think the parallel is the way to go. For the foreseeable future we should support both 6 and 7. Having dedicated branches for each version should also allow parallel automated builds and tests. If @AndysWorth https://github.com/AndysWorth pushes the required fixes to your branch we could merge things at the higher level. WDYT?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BIDS-Apps/freesurfer/issues/57#issuecomment-712918365, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABILLGCFXBB72Q3JWWB4FX3SLWRDHANCNFSM4N2FH5IQ .

AndysWorth commented 3 years ago

Great!

AndysWorth commented 3 years ago

I did a pull request just to update to the newer version of Freesurfer on the f_7 branch of Shotgunosine / freesurfer. I had plans on getting the fix for the -parallel bug in (I think they have a new recon-all script), but I don't know where to find it. If recon-all is called again after detecting a crash, it will finish properly.

Shotgunosine commented 3 years ago

@AndysWorth Might be worth an email to the freesurfer mailing list to see if you can track down the new recon-all script (or better yet, a URL for it). I'm not 100% sure how we'd get it into the images we build on circle, but I bet we could figure something out.

AndysWorth commented 3 years ago

I just emailed the list.

noxtoby commented 1 year ago

See also here

Remi-Gau commented 1 year ago

closed in #67