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

FIX: Move from smoothwm to white #806

Merged
merged 1 commit into from
May 18, 2023

Conversation

effigies
Copy link
Member

We have some places where wm is used for FreeSurfer surfaces instead of white, breaking regexes/path building when using white surfaces.

This PR also adjusts the (unused by anybody) gifti_surface_wf to convert white surfaces instead of smoothwm (see nipreps/smriprep#338), but that could be removed if we don't want to call that a bug fix.

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (9905f90) 62.28% compared to head (705517a) 62.28%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #806   +/-   ##
=======================================
  Coverage   62.28%   62.28%           
=======================================
  Files          49       49           
  Lines        5968     5968           
  Branches      883      883           
=======================================
  Hits         3717     3717           
  Misses       2071     2071           
  Partials      180      180           
Flag Coverage Δ
reportlettests ∅ <ø> (?)
unittests ∅ <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
niworkflows/anat/freesurfer.py 39.13% <ø> (ø)
niworkflows/interfaces/surf.py 47.46% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

I also wondered why we were using smoothwm rather than white, but this looks good to me. I do think it is a big enough change to warrant a minor bump though.

@effigies
Copy link
Member Author

Sounds good. I don't think we'll need this for any maintenance releases of downstream tools.

I think the reason was that I got white and smoothwm mixed up as to which was most refined ages ago.

@effigies effigies merged commit ecc9aa8 into nipreps:master May 18, 2023
effigies added a commit to nipreps/fmriprep that referenced this pull request Jun 6, 2023
…3011)

## Changes proposed in this pull request

1) Remove goodvoxels and CIFTI-specific code from `mri_vol2surf`
resampling workflow. fsnative and fsaverage will remain the same for
now.
2) Removes `fsaverage` as an implicit resampling target for
`--cifti-output`. `fsaverage` and `fsLR` are now entirely independent.
3) Create a subject-specific cortical ROI mask in GIFTI space. Not
currently output, as it's only needed internally; it's not a great mask
but is intended to be more liberal than the fsLR mask applied later.
4) Adds a number of `wb_command` commands, including an OpenMP mixin
that allows `OMP_NUM_THREADS` to be set in the environment. I think most
commands can take advantage, but I only tagged slow ones (take >10s)
that I could verify actually used them.
5) Depends on nipreps/smriprep#339 and
nipreps/niworkflows#806.


TODO:

* [x] Restore literate workflow code.
* [x] Restore saving goodvoxels mask, if desired.

## Documentation that should be reviewed

TODO

---

Closes #2990.
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