Affine Transform Refactoring

[mibaumgartner]

Short Description

This PR performs a refactoring of the affine transformations.
In the following X refers to 'scale', 'rotation', 'translation'

Proposed steps:

1. rename _format_X in utils.affine to _create_X and move them to transforms.function.affine

2. refactor _format_X/_create_X a little bit for easier understanding ...

   • this should fix a bug where it is not possible to define the rotation per axis
   • limit input to one of the following (no complete rotation matrices allowed): single scalar, scalar per
     dim, scalar per batch element, scalar per batch element per dim

3. Double check all doc strings and make them more precise

   • translation needs a unit: see later
   • rotation needs additional documentation in case of 3D where multiple dims are specified ... and should be fixed fro scale param to rotation param

4. double check if scale needs to be inverted

5. translation: I would like to pass in the number of pixels to shift (right now it only supports a float which shifts the image by that percentage -> shift depend on image size)

6. How to perform rotation around multiple axis (behaviour should replicate bg for compatibility and we may add additional options, e.g. different rotation orders/format, rorate around a unit vector ... )

7. additional unit tests where the behaviour is tested for some special cases: test intensity values not only shape

What do you think about this @justusschock ?

PR Checklist

PR Implementer

This is a small checklist for the implementation details of this PR.
If you submit a PR, please look at these points (don't worry about the RisingTeam
and Reviewer workflows, the only purpose of those is to have a compact view of
the steps)

If there are any questions regarding code style or other conventions check out our
summary.

• Implementation
  • 1.
  • 2.
  • 3.
  • 4.
  • 5.
  • 6.
  • 7.
• Docstrings & Typing
• Check __all__ sections and __init__
• Unittests (look at the line coverage of your tests, the goal is 100%!)
• Update notebooks & documentation if necessary
• Pass all tests
• Add the checksum of the last implementation commit to the Changelog

RisingTeam

RisingTeam workflow

• Add pull request to project (optionally delete corresponding project note)
• Assign correct label (if you don't have permission to do this, someone will do it for you.
  Please make sure to communicate the current status of the pr.)
• Does this PR close an Issue? (add closes #IssueNumber at the bottom if
  not already in description)

Reviewer

Reviewer workflow

• Do all tests pass? (Unittests, NotebookTests, Documentation)
• Does the implementation follow rising design conventions?
• Are the tests useful? (!!!) Are additional tests needed?
  Can you think of critical points which should be covered in an additional test?
• Optional: Check coverage locally / Check tests locally if GPU is necessary to execute

[justusschock]

I'm okay with 1.
In 2. it should be possible to specify an angle per axis in the current implementation if you pass it as a list. I'm okay with the other points.
3. And 4. Are also fine.
In 5. I'm not sure if this is really true. If it is, this is definitely a limitation introduced by PyTorch and wen can only encounter this if we manually compute the float each time. This requires more matrix computations on the other hand, even if the spatial size is same for all batches.
6. And 7. Are fine again

[mibaumgartner]

2. trafo = Rotate([0, 45, 0], degree=True, adjust_size=False) raises a ValueError from the if statement. (also tried a tensor and degree=False) [btw. this problem also occurs with translation and scaling, but maybe its only in 3D because the tests pass for the 2D case (Scaling is tested there with multiple axis)]
3. I did some testing in the notebook and the resulting values are matching my hypothesis (event though I think this is not mentioned inside the documentation). The computational burden should be negligible, its just a normalisation of values ... should be doable by overwriting assemble_matrix of the translation where the super class is called and the resulting matrix is normalised to the image shape

[justusschock]

This is an improvement over my initial code. I added some comments for discussion

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@2cb2d2d). Click here to learn what that means.
The diff coverage is 99.27%.

@@            Coverage Diff            @@
##             master      #31   +/-   ##
=========================================
  Coverage          ?   98.69%           
=========================================
  Files             ?       34           
  Lines             ?     1301           
  Branches          ?        0           
=========================================
  Hits              ?     1284           
  Misses            ?       17           
  Partials          ?        0

Flag	Coverage Δ
#unittests	98.69% <99.27%> (?)

Impacted Files	Coverage Δ
rising/transforms/affine.py	98.13% <100%> (ø)
rising/utils/affine.py	100% <100%> (ø)
rising/transforms/functional/affine.py	99.03% <98.5%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cb2d2d...42cfe91. Read the comment docs.

[justusschock]

Looks good in general, left some comments and questions

[justusschock]

Nother thing that came to my mind, but is not essential: We should include description text in our notebooks :)

[mibaumgartner]

Currently, the notebook is more like a visual test of the transformations. :) When the GridTransforms are ready, I will refactor it to be pretty :D