khanlab / hippunfold

BIDS App for Hippunfold (automated hippocampal unfolding and subfield segmentation)
https://hippunfold.readthedocs.io
MIT License
47 stars 12 forks source link

use naturalneighbour interpolation instead of linear barycentric #306

Open jordandekraker opened 2 days ago

jordandekraker commented 2 days ago

This PR adds only the best changes from https://github.com/khanlab/hippunfold/pull/300 and https://github.com/khanlab/hippunfold/pull/305.

Notably, from https://github.com/khanlab/hippunfold/pull/300 no changes are kept except those that make surface config easier to use. Also interpolation is performed in 256x128x16 unfolded space instead of 0-1x0-1x0-1 unfolded space. Other changes are no longer needed since they are handled in https://github.com/khanlab/hippunfold/pull/305.

From https://github.com/khanlab/hippunfold/pull/305, here are the major changes:

Naturalneighbor interpolation is in create_warps.py. This lovely interpolation function is fast and initial tests show it working extremely well, solving all issues in https://github.com/khanlab/hippunfold/pull/300 without the need for any postprocessing of surfaces. I tried many things to clean these surfaces, including different interpolation functions, but I believe this fix now finally solves the root cause.

This is a breaking change since outputs will now be slightly different (and better quality). I see no downsides or anticipated conflicts.

This is also now in-line with the original Matlab code that was used for interpolation (hippo_autotop)

Still doing a wet-run test.

jordandekraker commented 2 days ago

@akhanf please merge https://github.com/khanlab/hippunfold_deps/pull/12 before reviewing this PR

akhanf commented 2 days ago

Great! I'll try to have a look soon, and also build the design container (lock file needs updating first I think).

jordandekraker commented 2 days ago

OK i cleaned up the reviews. One more question: does https://github.com/khanlab/hippunfold_deps build the hippunfold_deps container automatically or do i need to trigger it?

akhanf commented 2 days ago

I've made a hippunfold_deps release (v0.5.1) which triggers the docker hub build. Will need to update the container version here to point to that too.