[RTM] Accept T2w images in recon-all #376

effigies · 2017-02-28T02:15:19Z

Starting to track progress on T2w incorporation.

Follow-up to #347.

effigies · 2017-03-01T14:26:09Z

I've run a test on sub-s483.

Nipype log excerpts:

170301-02:39:34,493 workflow INFO:
	 Executing node Reconstruction in dir: /scratch/workflow_enumerator/s483/t1w_preprocessing/Reconstruction
170301-02:39:34,498 workflow INFO:
	 Running: recon-all -autorecon1 -i /data/sub-s483/ses-1/anat/sub-s483_T1w.nii.gz -i /data/sub-s483/ses-2/anat/sub-s483_T1w.nii.gz -T2 /data/sub-s483/ses-1/anat/sub-s483_T2w.nii.gz -noskullstrip -hires -openmp 8 -parallel -subjid sub-s483 -sd /out/freesurfer

170301-07:04:35,101 workflow INFO:
	 Executing node Reconstruction2 in dir: /scratch/workflow_enumerator/s483/t1w_preprocessing/Reconstruction2
170301-07:04:35,167 interface INFO:
	 resume recon-all : recon-all -all -noskullstrip -hires -openmp 8 -parallel -subjid sub-s483 -sd /out/freesurfer -T2pial -nomotioncor -notalairach -nonuintensitycor -nonormalization

-T2 flag is being added correctly to recon-all -autorecon1, and -T2pial to recon-all -all.

Waiting to finish to verify that -T2pial runs correctly, but this should be in good shape.

I will note that for hi-res images the inflated surface looks a bit pointy in places, so we may want to reconsider adjusting mris_inflate -n in an expert file. I'm pretty sure the issue I was having with that before is that the output file needed to be marked as needed to avoid being deleted by remove_unnecessary_outputs.

chrisgorgo · 2017-03-01T15:03:46Z

It's freesurfer doing some form of skull stripping on the t2 you had to take care off?

…

On Mar 1, 2017 6:26 AM, "Chris Markiewicz" ***@***.***> wrote: I've run a test on sub-s483. Nipype log excerpts: 170301-02:39:34,493 workflow INFO: Executing node Reconstruction in dir: /scratch/workflow_enumerator/s483/t1w_preprocessing/Reconstruction 170301-02:39:34,498 workflow INFO: Running: recon-all -autorecon1 -i /data/sub-s483/ses-1/anat/sub-s483_T1w.nii.gz -i /data/sub-s483/ses-2/anat/sub-s483_T1w.nii.gz -T2 /data/sub-s483/ses-1/anat/sub-s483_T2w.nii.gz -noskullstrip -hires -openmp 8 -parallel -subjid sub-s483 -sd /out/freesurfer 170301-07:04:35,101 workflow INFO: Executing node Reconstruction2 in dir: /scratch/workflow_enumerator/s483/t1w_preprocessing/Reconstruction2 170301-07:04:35,167 interface INFO: resume recon-all : recon-all -all -noskullstrip -hires -openmp 8 -parallel -subjid sub-s483 -sd /out/freesurfer -T2pial -nomotioncor -notalairach -nonuintensitycor -nonormalization -T2 flag is being added correctly to recon-all -autorecon1, and -T2pial to recon-all -all. Waiting to finish to verify that -T2pial runs correctly, but this should be in good shape. I will note that for hi-res images the inflated surface looks a bit pointy in places, so we may want to reconsider adjusting mris_inflate -n in an expert file. I'm pretty sure the issue I was having with that before is that the output file needed to be marked as needed to avoid being deleted by remove_unnecessary_outputs. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#376 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp9SWVMeli3XRevd2IOtWUZp2ao99ks5rhYABgaJpZM4MN4eC> .

effigies · 2017-03-01T15:09:29Z

It uses brainmask.mgz, which we're injecting from ANTs. See https://surfer.nmr.mgh.harvard.edu/fswiki/ReconAllTableStableV6.0#line-116.

effigies · 2017-03-01T19:04:08Z

T2 coregisters fine. I want to try adjusting mris_inflate, because the inflated surface (with and without T2) is kind of pointy in places.

effigies · 2017-03-03T14:45:18Z

Upping mris_inflate -n definitely makes the inflated surface much smoother, so I'm including that.

No expert:

expert:

This PR is waiting on nipy/nipype#1857. The expert option for ReconAll is needed to mark the expert.opts file as not to be deleted.

Going to do a clean run on Sherlock and report total time.

chrisgorgo · 2017-03-03T15:02:39Z

Interesting. Is there an impact on how reconstructed surfaces look like in our HTML reports?

…

On Mar 3, 2017 6:46 AM, "Chris Markiewicz" ***@***.***> wrote: Upping mris_inflate -n definitely makes the inflated surface much smoother, so I'm including that. No expert: [image: no_expert] <https://cloud.githubusercontent.com/assets/83442/23555148/c7516a80-fff5-11e6-87fd-029512a7c830.png> expert: [image: expert] <https://cloud.githubusercontent.com/assets/83442/23555149/c7533f04-fff5-11e6-8665-a808cf4416ad.png> This PR is waiting on nipy/nipype#1857 <nipy/nipype#1857>. The expert option for ReconAll is needed to mark the expert.opts file as not to be deleted. Going to do a clean run on Sherlock and report total time. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#376 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkpw1anpGxGxk1Or2ETdiqQUSTLnZEks5riCd_gaJpZM4MN4eC> .

effigies · 2017-03-03T15:49:40Z

So reports uses ?h.white and ?h.pial, and I don't think those are affected by mris_inflate. I believe ?h.sphere is affected, which is used for subject-subject surface registration.

effigies · 2017-03-03T16:13:05Z

?h.white does depend on ?h.inflated in the smoothing stages, as do ?h.sphere. So this isn't purely aesthetic wrt the ?h.inflated surfaces.

effigies · 2017-03-03T17:22:46Z

I set this as RTM because it works. If you want to wait on Sherlock build, that's fine. (If I can't get Sherlock to work, I'll run on my laptop overnight and give you an 8-core estimate.)

oesteban · 2017-03-04T02:27:19Z

We also should wait for nipy/nipype#1857 to be merged, right?

effigies · 2017-03-04T02:33:55Z

Oh right. Yes, somehow I forgot that. I'll mark it [WIP] again. Once a final pin is set, I'll set it back to RTM.

effigies · 2017-03-04T12:12:21Z

SLURM Job_id=13250524 Name=sub-s483 Ended, Run time 09:38:39, COMPLETED, ExitCode 0

Saved in $SUBJECTS_DIR/<subj>/scripts/expert-options

Closes niprepsgh-382

effigies · 2017-03-04T19:04:30Z

fmriprep/workflows/anatomical.py

            t1w_ref = nib.load(t1w_list[0])
-            hires = max(t1w_ref.header.get_zooms()) < 1
+            hires = round(max(t1w_ref.header.get_zooms()), 1) < 1


Woops. Lost the thread in the rebase. Recap:

@oesteban suggested we set an explicit threshold, rather than round.

hires = np.all(np.array(t1w_ref.header.get_zooms()) - 1 < -tol)

Where tol here = 0.05.

If we're going to do that, I'd prefer to stick with the max formulation, and keep constants on the right-hand-side, e.g.:

hires = max(t1w_ref.header.get_zooms()) < 1 - 0.05

I'll do whatever the consensus is, but I think the round(max(zooms), 1) < 1 and max(zooms) < 1 - tol are the most readable options. Opinions, @chrisfilo?

chrisgorgo · 2017-03-04T19:23:51Z

I like the explicit threshold. Just make sure you cast everything to float!

…

On Sat, Mar 4, 2017 at 11:04 AM, Chris Markiewicz ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In fmriprep/workflows/anatomical.py <#376 (comment)>: > t1w_ref = nib.load(t1w_list[0]) - hires = max(t1w_ref.header.get_zooms()) < 1 + hires = round(max(t1w_ref.header.get_zooms()), 1) < 1 Woops. Lost the thread in the rebase. Recap: @oesteban <https://github.com/oesteban> suggested we set an explicit threshold, rather than round. hires = np.all(np.array(t1w_ref.header.get_zooms()) - 1 < -tol) Where tol here = 0.05. If we're going to do that, I'd prefer to stick with the max formulation, and keep constants on the right-hand-side, e.g.: hires = max(t1w_ref.header.get_zooms()) < 1 - 0.05 I'll do whatever the consensus is, but I think the round(max(zooms), 1) < 1 and max(zooms) < 1 - tol are the most readable options. Opinions, @chrisfilo <https://github.com/chrisfilo>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#376 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp4qUZK6zZ9lf-_TRbMW3z3YFJc6aks5ribW-gaJpZM4MN4eC> .

effigies · 2017-03-04T22:16:54Z

I don't see where casting is required... even if zooms are int (Nifti zooms are 'f4', anyway), numerical comparisons aren't going to be affected by an explicit cast. Am I missing something?

chrisgorgo · 2017-03-04T22:39:12Z

LGTM

effigies · 2017-03-06T03:23:47Z

Another run, following the nipype PR: Run time 09:22:15

oesteban · 2017-03-06T04:33:08Z

LGTM too

chrisgorgo · 2017-03-06T05:14:37Z

+1

…

On Mar 5, 2017 8:33 PM, "Oscar Esteban" ***@***.***> wrote: LGTM too — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#376 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOkp1AeppbnqtiuBqGs2XOJSPUluSvjks5ri4yEgaJpZM4MN4eC> .

effigies added feature freesurfer labels Feb 28, 2017

effigies force-pushed the reconall_t2 branch 2 times, most recently from 2c3eaf0 to 2e5d51e Compare March 1, 2017 02:41

effigies force-pushed the reconall_t2 branch 3 times, most recently from 280be13 to 454af19 Compare March 3, 2017 14:17

effigies changed the title ~~[WIP] Accept T2w images in recon-all~~ [RTM] Accept T2w images in recon-all Mar 3, 2017

effigies changed the title ~~[RTM] Accept T2w images in recon-all~~ [WIP] Accept T2w images in recon-all Mar 4, 2017

effigies added 7 commits March 4, 2017 12:01

Add T2 support to recon-all

030f61d

Simplify ReconConfig, pass T2 options as ports

688b747

Create expert file for recon-all -hires

2890650

Use hires/expert ports for recon-all

1181519

Expert options file need only be specified once

aca805b

Saved in $SUBJECTS_DIR/<subj>/scripts/expert-options

Round zooms to 0.1 for hires determination

b75249e

Closes niprepsgh-382

Update pins

ba2ebce

effigies force-pushed the reconall_t2 branch from a4ae247 to ba2ebce Compare March 4, 2017 17:02

Use mris_inflate ReconAll option

8bce49d

effigies commented Mar 4, 2017

View reviewed changes

Use tolerance-based hires check

da0bed2

effigies changed the title ~~[WIP] Accept T2w images in recon-all~~ [RTM] Accept T2w images in recon-all Mar 6, 2017

effigies merged commit 45fff14 into nipreps:master Mar 6, 2017

effigies deleted the reconall_t2 branch March 6, 2017 15:58

effigies mentioned this pull request Apr 18, 2017

[RTM] BF: Give detect_inputs default parameter values #453

Merged

[RTM] Accept T2w images in recon-all #376

[RTM] Accept T2w images in recon-all #376

Uh oh!

Conversation

effigies commented Feb 28, 2017

Uh oh!

effigies commented Mar 1, 2017

Uh oh!

chrisgorgo commented Mar 1, 2017 via email

Uh oh!

effigies commented Mar 1, 2017

Uh oh!

effigies commented Mar 1, 2017

Uh oh!

effigies commented Mar 3, 2017

Uh oh!

chrisgorgo commented Mar 3, 2017 via email

Uh oh!

effigies commented Mar 3, 2017

Uh oh!

effigies commented Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

effigies commented Mar 3, 2017

Uh oh!

oesteban commented Mar 4, 2017

Uh oh!

effigies commented Mar 4, 2017

Uh oh!

effigies commented Mar 4, 2017

Uh oh!

effigies Mar 4, 2017

Choose a reason for hiding this comment

Uh oh!

chrisgorgo commented Mar 4, 2017 via email

Uh oh!

effigies commented Mar 4, 2017

Uh oh!

chrisgorgo commented Mar 4, 2017

Uh oh!

effigies commented Mar 6, 2017

Uh oh!

oesteban commented Mar 6, 2017

Uh oh!

chrisgorgo commented Mar 6, 2017 via email

Uh oh!

Uh oh!

effigies commented Mar 3, 2017 •

edited

Loading