Skip to content

FS8 support#395

Open
demsarjure wants to merge 33 commits into
masterfrom
fs8
Open

FS8 support#395
demsarjure wants to merge 33 commits into
masterfrom
fs8

Conversation

@demsarjure

Copy link
Copy Markdown
Contributor

PR for easy comparison of FS8 required changes

Comment thread FreeSurfer/FreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/FreeSurferPipeline.sh Outdated
demsarjure and others added 3 commits May 6, 2026 08:54
@demsarjure demsarjure requested review from coalsont and glasserm May 6, 2026 09:00
@demsarjure

Copy link
Copy Markdown
Contributor Author

@coalsont @glasserm

The script should now support both FS6 and 7/8. Please review, I made the changes we discussed on Monday.

Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated

@glasserm glasserm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't understand some of these commits noted above. They would seem to break longitudinal processing.

@coalsont

coalsont commented May 7, 2026

Copy link
Copy Markdown
Member

I didn't understand some of these commits noted above. They would seem to break longitudinal processing.

I think he intends to do more stuff before a full review.

@demsarjure

Copy link
Copy Markdown
Contributor Author

I didn't understand some of these commits noted above. They would seem to break longitudinal processing.

@glasserm My bad. I did not fix the longitudinal script, longitudinal script is still in the FS8 only mode. Let me fix this today.

Comment thread FreeSurfer/FreeSurferPipeline.sh
@glasserm

Copy link
Copy Markdown
Contributor

Did we sort out the cause of the problem with the data sent to Erin?

@demsarjure

demsarjure commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Did we sort out the cause of the problem with the data sent to Erin?

I will share the data that was processed with the latest version of master + fs8 branch today, so you can check.

Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/FreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/FreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread FreeSurfer/LongitudinalFreeSurferPipeline.sh Outdated
Comment thread PostFreeSurfer/scripts/FreeSurfer2CaretConvertAndRegisterNonlinear.sh Outdated
Co-authored-by: Michael Harms <mharms@wustl.edu>
@mharms

mharms commented May 27, 2026

Copy link
Copy Markdown
Contributor

Big picture question: Don't we generally want to be using -hires with FS 8 (and probably FS 7 as well)? According to Gemini, if we don't include that, and we have acquired higher than 1.0 mm resolution, then the very first mri_convert step still does the classical "conform" to 1 mm step, and we lose our sub-millimeter resolution immediately and permanently.

@mharms

mharms commented May 27, 2026

Copy link
Copy Markdown
Contributor

Also, from what I can tell, what we've set up as the default for FS 8 in this PR is just a "standard" recon-all run, and doesn't take advantage of the newer deep learning-based tools (e.g., SynthMorph, SynthSeg). Is that the intent?

Note that there is a new recon-all-clinical.sh script, which defaults to using the DL tools, and is described as:

"Recon-all-like stream for clinical scans of arbitrary orientation/resolution/contrast"

@coalsont

coalsont commented May 28, 2026

Copy link
Copy Markdown
Member

Also, from what I can tell, what we've set up as the default for FS 8 in this PR is just a "standard" recon-all run, and doesn't take advantage of the newer deep learning-based tools (e.g., SynthMorph, SynthSeg). Is that the intent?

Note that there is a new recon-all-clinical.sh script, which defaults to using the DL tools, and is described as:

"Recon-all-like stream for clinical scans of arbitrary orientation/resolution/contrast"

In this context, "clinical" may mean "low quality". The context I am familiar with for SynthSeg is dealing with low-quality data, such as portable scanners, brain-extracted postmortem, etc.

I assume the "standard recon-all run" remains the MGH-recommended approach for high-quality data (if something is an improvement on high-quality data, they would change it in recon-all, right?).

@coalsont

coalsont commented May 28, 2026

Copy link
Copy Markdown
Member

Don't we generally want to be using -hires with FS 8 (and probably FS 7 as well)?

If I recall, that existed in fs6 as well, and didn't perform very well (and it has an effect on native mesh resolution, which may have caused things like inflated and/or sulc to be different for non-anatomical reasons). We would need to test -hires versus -conf2hires again, which may only be worth the effort if MGH thinks it has made substantial improvements to how it works.

@mharms

mharms commented May 28, 2026

Copy link
Copy Markdown
Contributor

I assume the "standard recon-all run" remains the MGH-recommended approach for high-quality data (if something is an improvement on high-quality data, they would change it in recon-all, right?).

Not necessarily. I don't know what their current recommendation is for "high-quality" data, but they could have opted to maintain the "traditional" approach as the default for now at least, solely for backwards-consistency considerations.

@mharms

mharms commented May 28, 2026

Copy link
Copy Markdown
Contributor

Don't we generally want to be using -hires with FS 8 (and probably FS 7 as well)?

If I recall, that existed in fs6 as well, and didn't perform very well (and it has an effect on native mesh resolution, which may have caused things like inflated and/or sulc to be different for non-anatomical reasons). We would need to test -hires versus -conf2hires again, which may only be worth the effort if MGH thinks it has made substantial improvements to how it works.

Per the usage notes, -hires was completely revamped going from FS 6 to 7. But not having either -hires or -conf2hires in FS 8 doesn't strike me as what we want as our default, from what I've read.

@demsarjure

Copy link
Copy Markdown
Contributor Author

Don't we generally want to be using -hires with FS 8 (and probably FS 7 as well)?

If I recall, that existed in fs6 as well, and didn't perform very well (and it has an effect on native mesh resolution, which may have caused things like inflated and/or sulc to be different for non-anatomical reasons). We would need to test -hires versus -conf2hires again, which may only be worth the effort if MGH thinks it has made substantial improvements to how it works.

Per the usage notes, -hires was completely revamped going from FS 6 to 7. But not having either -hires or -conf2hires in FS 8 doesn't strike me as what we want as our default, from what I've read.

I believe I resolved all issues above. The only "open" question is the use of -hires or -conf2hires in fs7 and 8. I do not have much experience here so I will defer to your guidance.

@coalsont

Copy link
Copy Markdown
Member

The question of what to do about "longitudinal" T2w with a single session also remains.

@demsarjure

Copy link
Copy Markdown
Contributor Author

The question of what to do about "longitudinal" T2w with a single session also remains.

True, waiting for Misha's input here.

@glasserm

Copy link
Copy Markdown
Contributor

With regard to the above comments:

  1. recon-all clinical is not relevant to the HCP Pipelines at this time. This is, for example, what we use in HMBA to make surfaces from lowres hyperfine scans.
  2. We need to test all the permutations (standard recon-all, -conf-to-hires, -hires
  3. With regard to a single session and longitudinal mode, I assume we would just use the T2w data that is available and let FreeSurfer do whatever it is going to do.

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.

5 participants