FEAT: Add 3D Radial Fourier Transform for medical image frequency analysis #8668
Conversation
…lysis - Implement RadialFourier3D transform for radial frequency analysis - Add RadialFourierFeatures3D for multi-scale feature extraction - Include comprehensive tests (20/20 passing) - Support for magnitude, phase, and complex outputs - Handle anisotropic resolution in medical imaging - Fix numpy compatibility and spatial dimension handling Signed-off-by: Hitendrasinh Rathod<hitendrasinh.data7@gmail.com> Signed-off-by: Hitendrasinh Rathod <Hitendrasinh.data7@gmail.com>
Hitendrasinhdata7
requested review from
KumoLiu,
Nic-Ma and
ericspod
as code owners
📝 WalkthroughWalkthroughAdds a new monai.transforms.signal subpackage and a radial_fourier module implementing RadialFourier3D (forward/inverse 3D FFT, radial coordinate computation, optional radial binning, magnitude/phase outputs) and RadialFourierFeatures3D (multi-resolution composition). Exposes these symbols in signal.init and the top-level transforms init; removes SignalRandShift from top-level exports. Adds tests/tests_radial_fourier.py covering shapes, dtypes, forward/inverse behavior, parameter validation, batching, multi-scale features, and API stability. Updates pyproject.toml pycln exclude patterns. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tests/test_radial_fourier.py (1)
76-88: Inverse transform test only checks shape, not reconstruction accuracy.Consider adding an assertion that the reconstructed data is close to the original input to validate correctness.
# Should have same shape self.assertEqual(reconstructed.shape, self.test_image_3d.shape) + + # Should approximately reconstruct original + self.assertTrue(torch.allclose(reconstructed, self.test_image_3d, atol=1e-5))monai/transforms/signal/radial_fourier.py (3)
137-144: Loop-based binning may be slow for large radial_bins.Consider vectorized binning using
torch.bucketizefor better performance, though current implementation is correct.
34-62: Docstring missingRaisessection.Per coding guidelines, docstrings should document raised exceptions.
Example: >>> transform = RadialFourier3D(radial_bins=64, return_magnitude=True) >>> image = torch.randn(1, 128, 128, 96) # Batch, Height, Width, Depth >>> result = transform(image) # Shape: (1, 64) + + Raises: + ValueError: If max_frequency not in (0.0, 1.0], radial_bins < 1, or both + return_magnitude and return_phase are False. """
30-31: Unused import.
spatialis imported but never used.-# Optional imports for type checking -spatial, _ = optional_import("monai.utils", name="spatial")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (4)
monai/transforms/__init__.py(1 hunks)monai/transforms/signal/__init__.py(1 hunks)monai/transforms/signal/radial_fourier.py(1 hunks)tests/test_radial_fourier.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
tests/test_radial_fourier.pymonai/transforms/signal/radial_fourier.pymonai/transforms/signal/__init__.pymonai/transforms/__init__.py
🧬 Code graph analysis (3)
tests/test_radial_fourier.py (1)
monai/transforms/signal/radial_fourier.py (3)
RadialFourier3D(34-279)RadialFourierFeatures3D(282-350)inverse(239-279)
monai/transforms/signal/__init__.py (1)
monai/transforms/signal/radial_fourier.py (2)
RadialFourier3D(34-279)RadialFourierFeatures3D(282-350)
monai/transforms/__init__.py (2)
monai/transforms/signal/array.py (1)
SignalRemoveFrequency(387-419)monai/transforms/signal/radial_fourier.py (2)
RadialFourier3D(34-279)RadialFourierFeatures3D(282-350)
🪛 Ruff (0.14.8)
monai/transforms/signal/radial_fourier.py
86-86: Avoid specifying long messages outside the exception class
(TRY003)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
90-90: Avoid specifying long messages outside the exception class
(TRY003)
166-166: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (7)
monai/transforms/__init__.py (1)
379-381: LGTM!New radial Fourier transforms are correctly imported and exported at the package level.
monai/transforms/signal/__init__.py (1)
11-17: LGTM!Module docstring and exports are correctly set up.
tests/test_radial_fourier.py (2)
25-136: Good test coverage for RadialFourier3D.Tests cover key scenarios including edge cases, type handling, and parameter validation.
138-193: Good test coverage for RadialFourierFeatures3D.Multi-scale feature extraction and numpy compatibility are well tested.
monai/transforms/signal/radial_fourier.py (3)
64-91: LGTM!Parameter validation is thorough and handles edge cases correctly.
239-279: LGTM!Inverse transform correctly handles the non-binned case with proper FFT shift operations.
343-348: Edge case: when transforms list is empty,output = imgmay cause issues.If
imgis a tensor and transforms is empty,output = imgis returned. Thenisinstance(img, np.ndarray)is False, sooutput.cpu().numpy()is never called. This is correct.However, if
imgis already a numpy array and transforms is empty, the function returns the numpy array directly without conversion, which is the expected behavior.
- Add device parameter to _compute_radial_coordinates to prevent CPU/GPU mismatch - Fix frequency mask expansion for multi-dimensional inputs - Add reconstruction accuracy test assertion (with proper magnitude+phase for inverse) - Add Raises section to docstring - Remove unused import - Address all review comments
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_radial_fourier.py (1)
35-35: Unused test fixture.
test_image_4dis created but never used in any test methods.Proposed fix
- self.test_image_4d = torch.randn(2, 1, 48, 64, 64, device=self.device) # Batch, Channel, D, H, W
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/transforms/signal/radial_fourier.pytests/test_radial_fourier.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
tests/test_radial_fourier.pymonai/transforms/signal/radial_fourier.py
🧬 Code graph analysis (1)
tests/test_radial_fourier.py (1)
monai/transforms/signal/radial_fourier.py (3)
RadialFourier3D(34-285)RadialFourierFeatures3D(288-356)inverse(245-285)
🪛 Ruff (0.14.10)
monai/transforms/signal/radial_fourier.py
90-90: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Avoid specifying long messages outside the exception class
(TRY003)
94-94: Avoid specifying long messages outside the exception class
(TRY003)
171-171: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: packaging
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: build-docs
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-py3 (3.10)
🔇 Additional comments (8)
monai/transforms/signal/radial_fourier.py (8)
88-94: Parameter validation is correct.All constraints are properly enforced. Static analysis flags long exception messages (TRY003), but these are clear and appropriate for
ValueError.
96-118: LGTM!Radial coordinate computation is correct. Device parameter properly addresses the prior CPU/GPU mismatch issue.
120-153: LGTM!Radial spectrum binning logic is correct. Averaging is properly computed for complex values.
187-219: LGTM!Radial binning with batch/channel preservation is correctly implemented. The reshape logic properly handles arbitrary batch dimensions.
220-228: LGTM!Frequency masking correctly handles variable non-spatial dimensions. The fix from prior review properly addresses dimension broadcasting.
230-243: LGTM!Output assembly correctly handles magnitude/phase extraction and concatenation. Type conversion preserves input format.
245-285: LGTM!Inverse transform correctly reconstructs spatial data from magnitude/phase. FFT inversion sequence is correct, and normalization is properly reversed.
288-356: LGTM!Multi-scale feature extraction correctly composes multiple RadialFourier3D transforms. Concatenation logic and type handling are correct. Empty bins_list case returns original image as expected.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monai/transforms/signal/radial_fourier.py (1)
88-93: Optional: Consider custom exception classes.Static analysis suggests custom exception classes for parameter validation instead of long messages in
ValueError. This is a minor style improvement and can be deferred.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/transforms/signal/radial_fourier.pytests/test_radial_fourier.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
tests/test_radial_fourier.pymonai/transforms/signal/radial_fourier.py
🧬 Code graph analysis (1)
tests/test_radial_fourier.py (2)
monai/transforms/signal/radial_fourier.py (3)
RadialFourier3D(33-284)RadialFourierFeatures3D(287-355)inverse(244-284)monai/utils/misc.py (1)
set_determinism(339-390)
🪛 Ruff (0.14.10)
monai/transforms/signal/radial_fourier.py
89-89: Avoid specifying long messages outside the exception class
(TRY003)
91-91: Avoid specifying long messages outside the exception class
(TRY003)
93-93: Avoid specifying long messages outside the exception class
(TRY003)
170-170: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: packaging
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: build-docs
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-pytorch (2.7.1)
🔇 Additional comments (8)
tests/test_radial_fourier.py (2)
41-138: Comprehensive test coverage.The test methods cover output shapes, complex inputs, normalization, inverse transforms, determinism, numpy compatibility, parameter validation, custom spatial dimensions, and batch processing. Well done.
141-196: LGTM - proper test structure and coverage.TestRadialFourierFeatures3D has correct setUp/tearDown structure and comprehensive tests for feature extraction scenarios.
monai/transforms/signal/radial_fourier.py (6)
95-117: Device handling correctly implemented.The device parameter addition resolves the previous device mismatch issue. Implementation is correct.
119-152: LGTM - radial binning logic is correct.The method correctly bins frequency components by radial distance and handles empty bins gracefully.
154-242: Core transform logic is sound.The FFT computation, radial coordinate calculation, binning logic, and output extraction are correctly implemented. Previous device and mask expansion issues have been resolved.
244-284: Inverse transform correctly implemented.The inverse handles the unbinned case with proper magnitude/phase reconstruction and FFT operations. Appropriately raises
NotImplementedErrorfor binned data that cannot be exactly inverted.
308-329: LGTM - multi-transform composition is correct.The initialization correctly creates RadialFourier3D instances for all combinations of bin counts and return types.
331-355: Feature concatenation handles edge cases well.The method correctly handles empty transforms, mixed numpy/tensor types, and preserves the input data type. Good defensive programming.
- Implements RadialFourier3D for anisotropic resolution normalization - Adds RadialFourierFeatures3D for multi-scale frequency analysis - Includes comprehensive test suite (20/20 passing) - Adds version compatibility for older PyTorch/Python versions - Follows MONAI transform conventions - Exclude transforms/__init__.py from pycln to avoid import removal
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
monai/transforms/signal/radial_fourier.py (5)
28-29: Remove dead code.Commented-out import adds no value.
Proposed fix
-# Optional imports for type checking -# spatial, _ = optional_import("monai.utils", name="spatial") # Commented out unused import
40-58: Docstring missing type annotations per Google style.Args section should include types for each parameter. Per coding guidelines, docstrings should describe each variable with its type.
Example format
Args: normalize (bool): if True, normalize the output by the number of voxels. return_magnitude (bool): if True, return magnitude of the complex result. return_phase (bool): if True, return phase of the complex result. radial_bins (Optional[int]): number of radial bins for frequency aggregation. If None, returns full 3D spectrum. max_frequency (float): maximum normalized frequency to include (0.0 to 1.0). spatial_dims (Union[int, Sequence[int]]): spatial dimensions to apply transform to.
123-123: Builtinsum()with generator of tensors starts from int 0.Python's
sum()initializes with0, so0 + tensorworks but is inefficient. Consider explicit stacking.Proposed fix
- radial = torch.sqrt(sum(c**2 for c in mesh)) + radial = torch.sqrt(torch.stack([c**2 for c in mesh]).sum(dim=0))
150-155: Loop-based binning may be slow for large volumes.Vectorized binning with
torch.bucketizeandscatter_reducewould improve performance. Acceptable for now but consider optimization if profiling shows bottleneck.
305-315: Docstring incomplete per coding guidelines.Missing type annotations in Args and no Raises section documented.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
monai/transforms/signal/radial_fourier.pypyproject.toml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
Review the Python code for quality and correctness. Ensure variable names adhere to PEP8 style guides, are sensible and informative in regards to their function, though permitting simple names for loop and comprehension variables. Ensure routine names are meaningful in regards to their function and use verbs, adjectives, and nouns in a semantically appropriate way. Docstrings should be present for all definition which describe each variable, return value, and raised exception in the appropriate section of the Google-style of docstrings. Examine code for logical error or inconsistencies, and suggest what may be changed to addressed these. Suggest any enhancements for code improving efficiency, maintainability, comprehensibility, and correctness. Ensure new or modified definitions will be covered by existing or new unit tests.
Files:
monai/transforms/signal/radial_fourier.py
🧬 Code graph analysis (1)
monai/transforms/signal/radial_fourier.py (3)
monai/networks/blocks/fft_utils_t.py (3)
fftshift(69-88)ifftshift(91-110)roll(45-66)monai/transforms/transform.py (1)
Transform(244-297)monai/utils/type_conversion.py (1)
convert_data_type(286-352)
🪛 Ruff (0.14.10)
monai/transforms/signal/radial_fourier.py
82-82: Avoid specifying long messages outside the exception class
(TRY003)
84-84: Avoid specifying long messages outside the exception class
(TRY003)
86-86: Avoid specifying long messages outside the exception class
(TRY003)
178-178: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: packaging
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: build-docs
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
🔇 Additional comments (3)
pyproject.toml (1)
39-39: LGTM!Appropriate exclusion to prevent pycln from stripping the new public API re-exports.
monai/transforms/signal/radial_fourier.py (2)
180-183: FFT order:ifftshiftbeforefftnassumes zero-frequency centered input.Standard practice is
fftnthenfftshift. Pre-shifting input is unusual. Verify this is intentional for your use case.
340-363: LGTM!Feature extraction and concatenation logic is sound.
| if self.return_magnitude and self.return_phase: | ||
| # Assuming they were concatenated along last dimension | ||
| split_idx = radial_tensor.shape[-1] // 2 | ||
| magnitude = radial_tensor[..., :split_idx] | ||
| phase = radial_tensor[..., split_idx:] | ||
| radial_tensor = torch.complex(magnitude * torch.cos(phase), magnitude * torch.sin(phase)) |
There was a problem hiding this comment.
Inverse logic assumes concatenation along last dim, but unbinned output isn't split that way.
When radial_bins=None, magnitude and phase each retain full spatial shape. Concatenating at line 243/247 along dim=-1 doubles the last spatial dimension. The inverse then incorrectly splits the last spatial dim in half.
This breaks spatial reconstruction.
Proposed fix
Store magnitude and phase separately or use a dedicated channel dimension:
if self.return_magnitude and self.return_phase:
- # Assuming they were concatenated along last dimension
- split_idx = radial_tensor.shape[-1] // 2
- magnitude = radial_tensor[..., :split_idx]
- phase = radial_tensor[..., split_idx:]
+ # Split along the concatenation dimension (last dim was doubled)
+ original_last_dim = original_shape[-1]
+ magnitude = radial_tensor[..., :original_last_dim]
+ phase = radial_tensor[..., original_last_dim:]
radial_tensor = torch.complex(magnitude * torch.cos(phase), magnitude * torch.sin(phase))Or better: return a dict/tuple instead of concatenating for invertible transforms.
Committable suggestion skipped: line range outside the PR's diff.
| else: | ||
| output = img |
There was a problem hiding this comment.
Returning original img when features is empty is unexpected.
If n_bins_list or return_types is empty, silently returning input could mask configuration errors. Consider raising ValueError.
Proposed fix
- else:
- output = img
+ else:
+ raise ValueError("No features extracted. Check n_bins_list and return_types configuration.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else: | |
| output = img | |
| else: | |
| raise ValueError("No features extracted. Check n_bins_list and return_types configuration.") |
🤖 Prompt for AI Agents
In monai/transforms/signal/radial_fourier.py around lines 357-358, the code
currently returns the original img when computed features are empty; change this
to validate inputs and raise a ValueError instead. Specifically, check early (or
before the branch) that n_bins_list and return_types produce at least one
feature (e.g., non-empty lists and compatible values), and if no features would
be produced raise ValueError with a clear message about invalid/empty
configuration rather than silently returning img; update any callers/tests
accordingly.
1 participant
Description
Implements 3D Radial Fourier Transform for medical imaging applications, addressing anisotropic resolution challenges and enabling rotation-invariant frequency analysis. This transform is specifically designed for medical images where voxel spacing often differs between axial, coronal, and sagittal planes (e.g., typical CT/MRI with different slice thickness vs in-plane resolution).
Medical Imaging Problem Addressed:
Key Features:
RadialFourier3D: Core transform for 3D radial frequency analysis with configurable radial binsRadialFourierFeatures3D: Multi-scale frequency feature extraction for comprehensive analysisTechnical Implementation:
monai/transforms/signal/radial_fourier.pytests/test_radial_fourier.py(20/20 passing, comprehensive coverage)Usage Examples: