Standardize Axes in Random Transforms. Add Random Axis to RandomMotion #1185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e _flip_image and _parse_restore into class
|
All checks pass now. |
fepegar
reviewed
Nov 16, 2024
| @@ -392,6 +402,15 @@ def axis_name_to_index(self, axis: str) -> int: | |||
| if not isinstance(axis, str): | |||
| raise ValueError('Axis must be a string') | |||
| axis = axis[0].upper() | |||
| if axis not in 'LRPAISTB': | |||
There was a problem hiding this comment.
Suggested change
| if axis not in 'LRPAISTB': | |
| if axis not in FLIP_AXIS: |
| for name, image in self.get_images_dict(subject).items(): | ||
| is_2d = image.is_2d() | ||
| axes = [a for a in self.axes if a != 2] if is_2d else self.axes | ||
| for name, _ in self.get_images_dict(subject).items(): |
There was a problem hiding this comment.
Suggested change
| for name, _ in self.get_images_dict(subject).items(): | |
| for name in self.get_images_dict(subject): |
| @@ -134,6 +142,7 @@ class Motion(IntensityTransform, FourierTransform): | |||
| simulate motion artifacts for data augmentation. | |||
|
|
|||
| Args: | |||
| axis: Integer representing the axis along which the simulated movements | |||
There was a problem hiding this comment.
I think this sentence is incomplete.
| reader.ReadImageInformation() | ||
| num_channels = reader.GetNumberOfComponents() | ||
| num_dimensions = reader.GetDimension() | ||
| try: |
There was a problem hiding this comment.
Are the changes in this method related to the goal of this PR?
| axes: Tuple[int, ...], | ||
| num_ghosts_range: Tuple[int, int], |
There was a problem hiding this comment.
What's the point of this change?
| @@ -211,35 +228,18 @@ def get_rigid_transforms( | |||
| ) -> List[sitk.Euler3DTransform]: | |||
| center_ijk = np.array(image.GetSize()) / 2 | |||
| center_lps = image.TransformContinuousIndexToPhysicalPoint(center_ijk) | |||
| identity = np.eye(4) | |||
| matrices = [identity] | |||
| ident_transform = sitk.Euler3DTransform() | |||
There was a problem hiding this comment.
Can you rename "ident" -> "identity"?
Comment on lines
-87
to
-95
| is_2d = subject.get_first_image().is_2d() | ||
| if is_2d and 2 in self.axes: | ||
| warnings.warn( | ||
| f'Input image is 2D, but "2" is in axes: {self.axes}', | ||
| RuntimeWarning, | ||
| stacklevel=2, | ||
| ) | ||
| self.axes = list(self.axes) | ||
| self.axes.remove(2) |
| super().__init__(**kwargs) | ||
| self.axes = _parse_axes(axes) | ||
| self.args_names = ('axes',) |
There was a problem hiding this comment.
I suspect the type checker is checking that this is a tuple somewhere, but I might be wrong.
Comment on lines
+100
to
+107
| @staticmethod | ||
| def flip_image(image, axes): | ||
| spatial_axes = np.array(axes, int) + 1 | ||
| data = image.numpy() | ||
| data = np.flip(data, axis=spatial_axes) | ||
| data = data.copy() # remove negative strides | ||
| data = torch.as_tensor(data) | ||
| image.set_data(data) |
| @@ -386,7 +385,7 @@ def guess_external_viewer() -> Optional[Path]: | |||
| def parse_spatial_shape(shape): | |||
| result = to_tuple(shape, length=3) | |||
| for n in result: | |||
| if n < 1 or n % 1: | |||
| if isinstance(n, (str, bytes)) or n < 1 or n % 1: | |||
There was a problem hiding this comment.
Do we also need to check for bytes?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #81
Description
This pull request standardizes the use of axes in random transforms. It also adds random axes options in the
RandomMotiontransform. Includes the following changes:to_tupleto see strings as singular values (this also required adding number of changes to checks for strings in areas where numbers are necessary). Tests all pass.parse_axesandensure_axes_indicesto baseTransformclass. Standardizes the use of axes in all transforms. Capable of accepting integer and specific string axis values. This is now used inRandomFlip,RandomMotion,RandomGhosting,RandomAnisotropy. Tests were added to check these additional axis values.RandomMotion. This now works properly on 2D images and can apply motion to any axis of 3D images. Also removed redundant change between matrix and transform.Checklist
CONTRIBUTINGdocs and have a developer setup (especially important arepre-commitandpytest)pytestmake htmlinside thedocs/folder