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

Registration V2 #102

Merged
merged 86 commits into from
Jan 19, 2024
Merged

Registration V2 #102

merged 86 commits into from
Jan 19, 2024

Conversation

edyoshikun
Copy link
Contributor

@edyoshikun edyoshikun commented Sep 22, 2023

This PR adds the following:

  • CLI for manual estimation of affine transform
  • CLI for optimizing an affine transformation with ants
  • CLI for applying a transformation using ants and cropping to overlapping region using Largest Internal Rectangle (LIR)

TODO:

  • merge Registration refator #118 (need to push updates from fry2)
  • test that registration works with keep_overhang: true @edyoshikun
  • now that estimate_affine, optimize_affine, and apply_affine all take source and targets dirpaths, we no longer need to keep source_shape_zyx and target_shape_zyx in the yml config file @edyoshikun
  • refactor registration and stabilization functions out of utils and into the registration and stabilization modules @ieivanov
  • clean up function names @ieivanov
  • update slurmkit examples
  • link issues closed by this PR

Future PRs:

  • rename target to fixed and source to moving to match convention in other image registration pipelines
  • rename estimate-affine, optimize-affine, and apply-affine CLI calls to estimate-registration, optimize-registration, and register
  • refactor process_single_position_v2 and apply_transform_to_zyx_and_save_v2 (and affine_transform) to work on arrays of consistent shape - either ZYX or CZYX, not both. Possibly move process_single_position and apply_transform_to_zyx_and_save to iohub (long with update_scale_metadata?). @talonchandler @ziw-liu
  • try to find better cropping algorithm (maybe https://github.com/planetlabs/maxrect or https://shapely.readthedocs.io/en/stable/reference/shapely.intersection.html)
  • check how apply_affine handles positions will all nans or zeros. Currently we convert nans to zeros, which should not be necessary

@edyoshikun edyoshikun changed the title initial commit adding tools for cropping and registering with ants registration using ants Sep 22, 2023
@ieivanov
Copy link
Collaborator

In the final version of the registration pipeline we should rotate the volumes by 90 deg such that they are in landscape mode rather than portrait mode (i.e. with larger width than height). Our brains process landscape aspect ratio more naturally and our monitors are built that way too. This is not critical now, but may affect some of the earlier design choices.

@edyoshikun edyoshikun requested review from talonchandler and ieivanov and removed request for talonchandler November 3, 2023 05:24
@edyoshikun edyoshikun marked this pull request as ready for review November 3, 2023 05:24
@edyoshikun
Copy link
Contributor Author

@talonchandler let's schedule some time to go over this. Some design choices that we should also think about:

  • Use scipy.ndimage vs ants registration
  • What yaml config files should we take as input and output for the estimate-affine and optimize-affine.
  • I don't want to complicate the functions to add the 90 deg rotation. If it's only for display, should we add this as a coordinateTransformMetadta()?

@talonchandler
Copy link
Contributor

talonchandler commented Nov 3, 2023

Excellent! @edyoshikun and I just finished a paired session and put together this list of (very superficial) TODO items. Thanks for all of your effort here @edyoshikun.

@edyoshikun I've tagged you on the ones that you're best suited to handle. I'll handle the rest.

  • update -ls and -lf flag names to -s and -t
  • @edyoshikun remove config from estimate-source-to-target-affine
  • remove saving of .svgs from estimate-source-to-target-affine
  • update optimize-affine docs, specify -o is YAML (edit: this is specified in the example docstring, so I think it's good)
  • optimize-affine flags for display and channel choice
  • @edyoshikun remove pre_affine_90degree... everywhere, handle output_shape parameter
  • add -v flag for optimize-affine
  • @edyoshikun add quick comment about how to move to scipy for GPU/cupy (not needed now)
  • comment on landscape-mode results. Can napari's transpose provide the rotations we need without changing data storage conventions?

edyoshikun and others added 13 commits January 5, 2024 09:13
* refactor apply_affine

* clean up

Co-authored-by: Eduardo Hirata-Miyasaki <eduardo.hirata@czbiohub.org>

* debug and add comments

* updating estimate_affine to match new yml

* don't start parallel pool for 1 iterable

* patch previous commit

* make sure the source_channel_used is first in the list for the optimizer

* make optimize_affine accept the new config parameters

* fix bug calling iohub info on paths with white spaces; rename target_channel_str

* better white space cli bug fix

* cleaner print statements

* don't use itertools.starmap

* clean up apply_affine print statements

* clean up optimize_affine

---------

Co-authored-by: Eduardo Hirata-Miyasaki <eduardo.hirata@czbiohub.org>
Co-authored-by: Eduardo Hirata-Miyasaki <edhiratam@gmail.com>
@ieivanov ieivanov linked an issue Jan 19, 2024 that may be closed by this pull request
@ieivanov ieivanov merged commit 0df6135 into main Jan 19, 2024
2 checks passed
@ieivanov ieivanov deleted the registration_v2 branch January 19, 2024 01:47
@ieivanov ieivanov restored the registration_v2 branch January 19, 2024 01:52
@ieivanov ieivanov deleted the registration_v2 branch August 13, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants