-
Notifications
You must be signed in to change notification settings - Fork 1.4k
FEAT: Add 3D Radial Fourier Transform for medical image frequency analysis #8668
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
base: dev
Are you sure you want to change the base?
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>
WalkthroughThis pull request introduces a new 3D Radial Fourier transform framework in MONAI. Two transforms are added: Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| def _compute_radial_coordinates(self, shape: tuple[int, ...]) -> torch.Tensor: | ||
| """ | ||
| Compute radial distance from frequency domain center. | ||
|
|
||
| Args: | ||
| shape: spatial dimensions (D, H, W) or (H, W, D) depending on dims order. | ||
|
|
||
| Returns: | ||
| Tensor of same spatial shape with radial distances. | ||
| """ | ||
| # Create frequency coordinates for each dimension | ||
| coords = [] | ||
| for dim_size in shape: | ||
| # Create frequency range from -0.5 to 0.5 | ||
| freq = torch.fft.fftfreq(dim_size) | ||
| coords.append(freq) | ||
|
|
||
| # Create meshgrid and compute radial distance | ||
| mesh = torch.meshgrid(coords, indexing="ij") | ||
| radial = torch.sqrt(sum(c**2 for c in mesh)) | ||
|
|
||
| return radial |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential device mismatch: radial coordinates created on CPU.
_compute_radial_coordinates creates tensors on CPU. When the input is on GPU, this will cause device mismatch in _compute_radial_spectrum at line 139 where radial_coords is compared with bin_edges (which is on spectrum.device).
Proposed fix
Pass device to the method and create tensors on correct device:
- def _compute_radial_coordinates(self, shape: tuple[int, ...]) -> torch.Tensor:
+ def _compute_radial_coordinates(self, shape: tuple[int, ...], device: torch.device = None) -> torch.Tensor:
"""
Compute radial distance from frequency domain center.
Args:
shape: spatial dimensions (D, H, W) or (H, W, D) depending on dims order.
+ device: device to create tensor on.
Returns:
Tensor of same spatial shape with radial distances.
"""
# Create frequency coordinates for each dimension
coords = []
for dim_size in shape:
# Create frequency range from -0.5 to 0.5
- freq = torch.fft.fftfreq(dim_size)
+ freq = torch.fft.fftfreq(dim_size, device=device)
coords.append(freq)Then update the call site at line 179:
- radial_coords = self._compute_radial_coordinates(spatial_shape)
+ radial_coords = self._compute_radial_coordinates(spatial_shape, device=img_tensor.device)📝 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.
| def _compute_radial_coordinates(self, shape: tuple[int, ...]) -> torch.Tensor: | |
| """ | |
| Compute radial distance from frequency domain center. | |
| Args: | |
| shape: spatial dimensions (D, H, W) or (H, W, D) depending on dims order. | |
| Returns: | |
| Tensor of same spatial shape with radial distances. | |
| """ | |
| # Create frequency coordinates for each dimension | |
| coords = [] | |
| for dim_size in shape: | |
| # Create frequency range from -0.5 to 0.5 | |
| freq = torch.fft.fftfreq(dim_size) | |
| coords.append(freq) | |
| # Create meshgrid and compute radial distance | |
| mesh = torch.meshgrid(coords, indexing="ij") | |
| radial = torch.sqrt(sum(c**2 for c in mesh)) | |
| return radial | |
| def _compute_radial_coordinates(self, shape: tuple[int, ...], device: torch.device = None) -> torch.Tensor: | |
| """ | |
| Compute radial distance from frequency domain center. | |
| Args: | |
| shape: spatial dimensions (D, H, W) or (H, W, D) depending on dims order. | |
| device: device to create tensor on. | |
| Returns: | |
| Tensor of same spatial shape with radial distances. | |
| """ | |
| # Create frequency coordinates for each dimension | |
| coords = [] | |
| for dim_size in shape: | |
| # Create frequency range from -0.5 to 0.5 | |
| freq = torch.fft.fftfreq(dim_size, device=device) | |
| coords.append(freq) | |
| # Create meshgrid and compute radial distance | |
| mesh = torch.meshgrid(coords, indexing="ij") | |
| radial = torch.sqrt(sum(c**2 for c in mesh)) | |
| return radial |
🤖 Prompt for AI Agents
In monai/transforms/signal/radial_fourier.py around lines 92 to 113,
_compute_radial_coordinates currently creates frequency tensors on CPU which
causes device-mismatch when used with GPU tensors; modify the method to accept a
device (and optionally dtype) parameter and create all frequency coordinate
tensors and the meshgrid on that device so the returned radial tensor lives on
the same device as the spectrum, and update the call site at line 179 to pass
spectrum.device (and spectrum.dtype if needed) when invoking
_compute_radial_coordinates.
| # Apply frequency mask if max_frequency < 1.0 | ||
| if self.max_frequency < 1.0: | ||
| freq_mask = radial_coords <= (self.max_frequency * 0.5) | ||
| # Expand mask to match spectrum dimensions | ||
| for _ in range(len(self.spatial_dims)): | ||
| freq_mask = freq_mask.unsqueeze(0) | ||
| spectrum = spectrum * freq_mask |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frequency mask expansion may be incorrect for inputs with more than 3 non-spatial dimensions.
The loop adds len(spatial_dims) (always 3) leading dimensions, but should add dimensions equal to len(spectrum.shape) - len(spatial_shape) to properly broadcast.
Proposed fix
if self.max_frequency < 1.0:
freq_mask = radial_coords <= (self.max_frequency * 0.5)
# Expand mask to match spectrum dimensions
- for _ in range(len(self.spatial_dims)):
+ n_non_spatial = len(spectrum.shape) - len(spatial_shape)
+ for _ in range(n_non_spatial):
freq_mask = freq_mask.unsqueeze(0)
spectrum = spectrum * freq_maskCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In monai/transforms/signal/radial_fourier.py around lines 216 to 222, the code
always unsqueezes the radial frequency mask len(self.spatial_dims) times
(effectively 3), which is incorrect when spectrum has more than 3 non-spatial
leading dimensions; compute num_leading = len(spectrum.shape) -
len(self.spatial_dims) and unsqueeze the mask that many times (or
reshape/prepend that many singleton dimensions) so the mask broadcasts correctly
to spectrum before multiplying.
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: