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

Full lazy resampling implementation #6011

Closed
wants to merge 38 commits into from
Closed

Full lazy resampling implementation #6011

wants to merge 38 commits into from

Conversation

atbenmurray
Copy link
Contributor

@atbenmurray atbenmurray commented Feb 16, 2023

Refactor of compose, transforms and related functionality deliver full lazy resampling functionality. This PR reworks the spatial transforms to decouple the description of the transform from the execution of the transform, as per the design specified in #4855. It also decouples compose from various sources of modification to a sequence of transforms that may occur due to multi-sampling, lazy execution, caching, and potential other augmentations, through the provision of a compose compiler.

Description

Key: n - not done, p - partial, c - code complete

Refactored transforms:

  • [p] spatial
    • [n] SpatialResample/ResampleToMatch
    • [c] Spacing
    • [p] Orientation
    • [c] Flip/RandFlip/RandAxisFlip
    • [c] Resize
    • [c] Rotate/RandRotate
    • [c] Zoom/RandZoom
    • [c] Rotate90/RandRotate90
    • [n] Affine/RandAffine
    • [n] AffineGrid/RandAffineGrid
    • [n] Resample (NA?)
    • [p] Rand2DElastic
    • [p] Rand3DElastic
    • [n] GridDistortion/RandGridDistortion
  • [p] croppad
    • [c] CropPad (new)
    • [c] RandomCropPad (new)
    • [c] RandomCropPadMultiSample (new)
    • [n] Others

Compose

  • [p] Modified compose
  • [c] Compose compiler
    • [c] lazy compilation
    • [p] multi-sample compilation (may not be required)
    • [p] cached compilation

Consolidated resample

  • [p] Resampler
    • [p] matrix analysis
    • [p] matrix feature extraction
    • [n] orthogonal resample path
    • [n] affine matrix resample
    • [c] affine grid resample

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

…es, manually merged with updated dev

Signed-off-by: Ben Murray <ben.murray@gmail.com>
…hat the default hasn't been modified yet). Adding tests for functional resize

Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
…skeleton of consolidated resampler; bug fixes on randomizers

Signed-off-by: Ben Murray <ben.murray@gmail.com>
…ies (moved traits to traits.py, moved transforms.utils classes that call back into compose.py out of transforms.utils.py); minor fix to apply_transforms

Signed-off-by: Ben Murray <ben.murray@gmail.com>
… new compose inverse; minor miscellaneous fixes
@atbenmurray atbenmurray marked this pull request as draft February 16, 2023 09:54
… across from old branch; bug fix for swapped parameters in DiscreteRandomizer

Signed-off-by: Ben Murray <ben.murray@gmail.com>
@atbenmurray atbenmurray changed the title Lr development Full lazy resampling refactor Feb 16, 2023
@atbenmurray atbenmurray changed the title Full lazy resampling refactor Full lazy resampling implementation Feb 16, 2023
@Nic-Ma
Copy link
Contributor

Nic-Ma commented Feb 16, 2023

Hi @atbenmurray ,

Sorry I didn't follow all the previous discussions, could you please help provide a typical usage example of your proposal?
I don't quite understand how to use the ComposeCompiler.
Seems it refactored several important components of MONAI: multi-sampling, caching and lazy-resampling? I think we should avoid breaking change as we are already in v1.1, especially the user API.

Thanks in advance.

@atbenmurray
Copy link
Contributor Author

atbenmurray commented Feb 16, 2023 via email

…unctions, consolidated resampler work including unit_translate test function

Signed-off-by: Ben Murray <ben.murray@gmail.com>
…to MetaMatrix initialisation matrix identification code

Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
…rng; bug fixes for rand spatial array lazy transforms
…lasses

Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
Signed-off-by: Ben Murray <ben.murray@gmail.com>
@wyli
Copy link
Contributor

wyli commented Mar 16, 2023

could you please clean up the unused LR related branches @atbenmurray ? https://github.com/Project-MONAI/MONAI/branches

@atbenmurray
Copy link
Contributor Author

Yep, there are a few that can go

@wyli
Copy link
Contributor

wyli commented Mar 17, 2023

Yep, there are a few that can go

sure, thanks, all of us use forks to develop instead of creating branches in this primary repo. I'll make a backup of these branches and remove them.

…ing code

Signed-off-by: Ben Murray <ben.murray@gmail.com>
Renamed lazy_evaluation parameters to lazy
Added tests for flip function
Removed redundant Croppadd class

Signed-off-by: Ben Murray <ben.murray@gmail.com>
functions to be called by resample

Signed-off-by: Ben Murray <ben.murray@gmail.com>
…ive resampler

Signed-off-by: Ben Murray <ben.murray@gmail.com>
@wyli
Copy link
Contributor

wyli commented Apr 23, 2023

Closing this PR, as the initial prototype design is already on the dev branch, and this branch diverged. (This branch is backed up here https://github.com/wyli/MONAI/tree/lr_development)

@wyli wyli closed this Apr 23, 2023
@wyli wyli deleted the lr_development branch April 23, 2023 12:11
@atbenmurray
Copy link
Contributor Author

atbenmurray commented Apr 23, 2023

@wyli. This pr and branch should not have been closed and deleted. This is ongoing work that was paused to ensure that the 1.2 lazy resampling was ready for release. This could easily have been handled by asking me if this branch was still relevant. This work is still the v+1 (full) lazy resampling implementation and is still the ultimate design goal for it.

@atbenmurray atbenmurray restored the lr_development branch April 23, 2023 13:41
@atbenmurray atbenmurray reopened this Apr 23, 2023
@wyli
Copy link
Contributor

wyli commented Apr 23, 2023

Could you please start from your fork?

@wyli wyli closed this Apr 23, 2023
@atbenmurray
Copy link
Contributor Author

@wyli, if you would like me to move this branch to my fork for further development (as I understand we are moving a purely fork-based contribution model) then I am happy to do so. I will then delete this branch subsequent to doing so.

@wyli wyli deleted the lr_development branch April 23, 2023 13:43
@atbenmurray
Copy link
Contributor Author

atbenmurray commented Apr 23, 2023

Please leave the development branch intact for the time being, until I have ensured that the fork is intact and up to date.

@atbenmurray atbenmurray restored the lr_development branch April 23, 2023 13:44
@wyli
Copy link
Contributor

wyli commented Apr 23, 2023

Sure, I'll keep the branch till the end of this month. And then remove it to avoid any confusions.

@atbenmurray
Copy link
Contributor Author

I'll remove it later today when I am happy that the fork is fine.

@wyli wyli deleted the lr_development branch May 1, 2023 08:33
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