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

ENH: Refactor itk-dreg abstract interface for plugin methods #2

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

tbirdso
Copy link
Collaborator

@tbirdso tbirdso commented Oct 9, 2023

Proposal to refactor the itk-dreg platform interface to better support
extended registration methods within a dask-based framework:

  • Moves interfaces into import-able itk_dreg Python module
  • Restructures base types and abstract methods
  • Introduces helper types for working with dask blocks with metadata
  • Proposes updates to "register` interface for block-based registration
  • Proposes abstract interfaces for ITK image stream reader construction
    and block registration postprocessing.

Discussion and feedback are welcome.

@tbirdso tbirdso requested a review from thewtex October 9, 2023 15:18
@tbirdso tbirdso changed the title ENH: Refactor itk-dreg abstract interface for pluggable methods ENH: Refactor itk-dreg abstract interface for plugin methods Oct 9, 2023
@tbirdso
Copy link
Collaborator Author

tbirdso commented Oct 9, 2023

Feedback from DRMNI discussion:

  • If a pairwise registration method makes an inverse transform available, we should propagate it alongside the forward transform result
  • We should consider how large point clouds / meshes could be streamed with each block, though implementation could be pushed to a later date
  • Block overlap may be particularly helpful in dealing with 1) signal content border conditions in registration and 2) deformation field edge blending for output composition, consider pursuing this sooner rather than later

@tbirdso
Copy link
Collaborator Author

tbirdso commented Oct 10, 2023

Addressed feedback from today:

  1. Inverse transforms are now available as an optional field in block pairwise registration results. It is the duty of the registration and postprocessing implementations to determine whether or not to handle this field. Registration methods such as ANTs or ICON that make this inverse transform available by default can simply pass the transform out with each block result. Postprocessing methods that opt to populate the inverse transform for overall stage results may need to take extra care to ensure that a composition of forward transforms is equally valid for the inverse. For simplicity, we will probably opt to not attempt to find consensus of both fixed and inverse transforms in our initial implementation with ITKElastix.
  2. After considering further, it is still my opinion that method-specific data such as large clouds or meshes should be handled by the user-supplied registration method. itk-dreg infrastructure should try to explicitly handle every possible type of input data for registration. Appropriate input can be piped to the registration method under the existing interface proposal with **kwargs, so no changes are required.
  3. I've updated the proposal to provide the source and target requested regions to the registration method to reflect "what the buffered image region would have been if the image were not padded". The user-supplied registration method can compare the input image requested region and buffered region to get the difference attributable to some itk-dreg padding factor.

@tbirdso tbirdso marked this pull request as ready for review October 10, 2023 12:41
@thewtex thewtex requested a review from HastingsGreer October 11, 2023 11:55
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Very nice!

src/itk_dreg/base/image_block_interface.py Outdated Show resolved Hide resolved
src/itk_dreg/base/itk_typing.py Outdated Show resolved Hide resolved
src/itk_dreg/base/itk_typing.py Outdated Show resolved Hide resolved
src/itk_dreg/base/image_block_interface.py Outdated Show resolved Hide resolved
src/itk_dreg/base/registration_interface.py Show resolved Hide resolved
src/itk_dreg/base/registration_interface.py Outdated Show resolved Hide resolved
src/itk_dreg/base/registration_interface.py Outdated Show resolved Hide resolved
src/itk_dreg/base/registration_interface.py Outdated Show resolved Hide resolved
@tbirdso
Copy link
Collaborator Author

tbirdso commented Oct 17, 2023

@thewtex ping for re-review

Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

Loving it! 😍

@tbirdso
Copy link
Collaborator Author

tbirdso commented Oct 18, 2023

Squashed changes

EDIT: Also corrected the ReduceResultsMethod.__call__ docstring to indicate that block results are reduced with reference to the "fixed" image and not the "moving" image.

Refactors the proposed `itk-dreg` platform interface to better support
extended registration methods within a `dask`-based framework:
- Moves interfaces into `import`-able `itk_dreg` Python module
- Restructures base types and abstract methods
- Introduces helper types for working with dask blocks with metadata
- Proposes updates to "register` interface for block-based registration
- Proposes abstract interfaces for ITK image stream reader construction
  and block registration postprocessing.
Add simple `pytest` infrastructure to validate that `itk_dreg` module
scripts load without error.
@tbirdso tbirdso merged commit 5fd71b9 into InsightSoftwareConsortium:main Oct 18, 2023
1 check passed
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