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

Marmunfold #264

Closed
wants to merge 31 commits into from
Closed

Marmunfold #264

wants to merge 31 commits into from

Conversation

akhanf
Copy link
Member

@akhanf akhanf commented Oct 2, 2023

Marmoset hippocampal unfolding

  • Adds a new template-based segmentation (alternative to nnunet) option, --use-template-seg
  • Adds two new templates, MBMv2 (from ex vivo single hemisphere data), and MBMv3 (from in vivo group template) (using images from here: https://marmosetbrainmapping.org/atlas.html)

e.g. these options can be used to segment/unfold a marmoset in vivo brain MRI: --template MBMv3 --use-template-seg --crop-native-res 0.05x0.05x0.05mm

The manual tissue dsegs were created by manual correction and a number of additional registration/image processing steps (thanks @Bradley-Karat @jordandekraker for the initial MBMv2 segmentation in the native dti space)

This also adds a simple snakefile for producing the coords for the templates from these dsegs (since those are required for initialization), was done manually in Matlab before I believe..

Note 1: Haven't tested this extensively on new (individual marmoset) datasets yet, but I am also creating a repo (marmunfold?) with hippunfold results run on the templates themselves, for use in group studies (eg that already use the MBMv3 space), specifically for an Everling lab collaboration

Note 2: the tissue dsegs could use a little bit more tweaking, there is a discontinuity in the head that needs to be resolved. @Bradley-Karat or @jordandekraker would be great if you are able to have a look, I might run out of time to get to that..

@Bradley-Karat
Copy link
Collaborator

My bad, accidentally closed the PR... I should be able to take a look and tweak the dsegs/deal with the discontinuities in the head shortly

@akhanf
Copy link
Member Author

akhanf commented Oct 3, 2023 via email

@jordandekraker
Copy link
Collaborator

Sorry, I've been out of town but I'm really interested in what you're adding in this branch!

I really like the new --use_template_seg workflow, and I wonder if maybe we should consider adding a readthedocs page on it. That is, we should note that this uses registration as the backbone rather than nnUNet, and so it makes a pretty strong assumption that interindividual differences are small and/or easily captured by a relatively smooth warp field (ie. not concerned about different numbers of digitations).

It may also be a good place to add in some of the development I did alongside Nicole Eichert here
I will talk to her about adding her manual segmentation and data to our template list!

Perhaps the readthedocs page should be "nonhuman data" so we can include macaque, marmoset, and maybe eventually rat and mouse too. In Nicole's paper we also argued that unfolded space was ideal for cross-species comparison since topology is a good basis for homology.

I haven't gotten to look closely at the marmoset manual segmentation yet, but one thing we noted in the macaque is that we had to modify the subiculum-EC boundary. Using the heuristic of the superior-medial most extent of white matter below the hippocampus turned out to include some EC, which we found out since it had much higher T1w than subiculum. @Bradley-Karat let me know if you want me to have a look at your latest segmentations!

@jordandekraker jordandekraker added enhancement New feature or request major New major feature labels Oct 10, 2023
@jordandekraker
Copy link
Collaborator

Note 3: currently the --use_template_seg does not employ unfolded space registration. If we have strong prior thickness, curvature, and gyrification maps then we could employ unfolded space registration.

@akhanf
Copy link
Member Author

akhanf commented Oct 12, 2023

Yes would be good to include documentation once we iron things out.

Could consider maybe a technical report detailing these new features and templates, atlases etc? The marm application is likely going to be its own paper as well.

Also would be great to make all the templates consistent with this. There may be some additional images we could include too (eg the T2w image associated with tpl-upenn).

We should also think about having a way for creating unfolded atlases, perhaps as a CLI option to make use of hippunfold outputs.

Maybe a minor point for now, but the git repo is getting really large with all the template files, in another branch I was playing around with workflows for pushing the resources to zenodo and downloading on the fly. But maybe can sort that out separately if needed..

Finally, I will actually have very little time to work on these things in the near term, have a 1 week old at home now!

@jordandekraker
Copy link
Collaborator

accidentally committed to this branch instead of https://github.com/khanlab/hippunfold/tree/osf-resources (twice). reverting those commits

@jordandekraker
Copy link
Collaborator

Please put any updates to manual segmentations in the new: https://osf.io/v8acf/
This may change the URL hashes in config/snakebids, so i will try to add a utility to automatically extract URLs

akhanf and others added 18 commits November 16, 2023 11:04
still a WIP

TODO: clean-up config and CLI for specifying templates -- right now
--template and --template_files is used to select the rigid/affine reg template, and
--inject_template and --inject_files is used to select the template shape injection or
registration-based segmentation template (for use_template_seg).. could
harmonize the structure for files, and keep separate flags..
 only --template and --use-template-seg are needed now - prints a
warning if use_template_seg cannot be enabled (e.g. if dseg and coords
don't both exist)..
this template is a group average and better suited for template-based
segmentation of new datasets instead of the ex vivo MBMv2.

Note: hipp segmentation might need some additional tweaks..
@kaitj
Copy link
Contributor

kaitj commented Nov 16, 2023

@jordandekraker - It looks like whatever the issue was with the doc check / build was fixed some time in the last couple of weeks. You may want to take a quick look to make sure your changes are still intact - I didn't do anything here other then resolve the merge conflict.

@jordandekraker
Copy link
Collaborator

Currently this is failing because downloads.smk only supports one values for config["atlas"], but our test cases try multiple atlases. I can't seem to work out the wildcard uses for allowing multiple atlases though. Take a look @akhanf, or otherwise I'll try later this week.

Will push the CIVM marmoset brain soon

@akhanf
Copy link
Member Author

akhanf commented Feb 14, 2024

Ah ok -- the way you've implemented the downloading rules/functions you have the output files needing to be defined for each wildcard.. That way it makes it impossible to use a wildcard instead of the config['atlas'] directly.

The better way to solve this is to not put the zipfile contents explicitly as snakemake outputs, and instead make use of the standardized set of keys in the config (e.g. label_nii, subfields_list, label_list, etc..) to grab the appropriate files from the unzipped folder. Can either extract each zip to a folder and use directory(), or could have a rule that pulls files directly out of the zips..

I'll see if I can quickly refactor it that way --

@akhanf
Copy link
Member Author

akhanf commented Feb 14, 2024

having a look at all the changes -- this branch is pretty messy since it has the new template-based segmentation, change in downloading, and new marm templates/atlases -- I'm going to make a new branch to specifically deal with the download change (since alot of that will change..), then rebase another new branch with the templateseg changes..

@jordandekraker
Copy link
Collaborator

I may be close to a fix! Will push my local changes shortly

@akhanf
Copy link
Member Author

akhanf commented Feb 14, 2024

Ah I've started redoing things in another branch already but feel free to commit here.

@jordandekraker
Copy link
Collaborator

OK I added the details about the CIVM template plus it is uploaded to OSF already.

I have local uncommitted changes for handling the downloads, and it actually is similar to your suggested solution (ie. zipped outputs are defined in a dict which is then forwarded to rules that rely on downloads). I still can't quite get it working end-to-end though. I'll commit them on a separate branch to here and if we use it then great and if not the i'm happy to appl your branch instead!

- ensure atlas is a list in the config
- create download/unzip rules for each atlas, using a for loop over the
atlases (since output files need to be specified for each, but wildcards
cannot be used on outputs.
- change config['atlas'] back to wildcards.atlas whenever it is used to
get input files
- use pathlib Path() / file syntax instead of os.path.join (pathlib is
preferred now, may change this throughout in the future)..
- TODO: linting, snakefmt etc
@akhanf
Copy link
Member Author

akhanf commented Feb 15, 2024

ok just made the minimal fixes here to get atlas wildcards working again, as I didn't really have time to redo things in the separate branch..

@akhanf
Copy link
Member Author

akhanf commented Feb 15, 2024

nevermind looks like I didn't address everything.. Gonna go back to trying in a separate branch with a non-broken version..

@akhanf
Copy link
Member Author

akhanf commented Feb 15, 2024

Alright have a look at #277, that has the changes to enable download of resources.. I'm also going to look into removing the old deleted files from the git history, looks like theres a tool for that..

@akhanf
Copy link
Member Author

akhanf commented Feb 15, 2024

I'll make another branch with just the marmunfold changes now

@akhanf
Copy link
Member Author

akhanf commented Apr 25, 2024

since the replacement for this #281 has been merged now, I'm going to close this PR and delete the branch

@akhanf akhanf closed this Apr 25, 2024
@akhanf akhanf deleted the marmunfold branch April 25, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants