Fix code and improve unit tests to prevent fail-over to the default decoders when nvimgcodec is tested #573
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,19 +1,126 @@ | ||||||
| import logging | ||||||
| import time | ||||||
| from pathlib import Path | ||||||
| from typing import TYPE_CHECKING, Any, Iterator, cast | ||||||
|
|
||||||
| import numpy as np | ||||||
| import pytest | ||||||
| from pydicom import dcmread | ||||||
| from pydicom.data import get_testdata_files | ||||||
|
|
||||||
| from monai.deploy.operators.decoder_nvimgcodec import ( | ||||||
| SUPPORTED_DECODER_CLASSES, | ||||||
| SUPPORTED_TRANSFER_SYNTAXES, | ||||||
| _is_nvimgcodec_available, | ||||||
| register_as_decoder_plugin, | ||||||
| unregister_as_decoder_plugin, | ||||||
| ) | ||||||
|
|
||||||
| try: | ||||||
| from PIL import Image as PILImage | ||||||
| except Exception: # pragma: no cover - Pillow may be unavailable in some environments | ||||||
| PILImage = None # type: ignore[assignment] | ||||||
|
|
||||||
| if TYPE_CHECKING: | ||||||
| from PIL.Image import Image as PILImageType | ||||||
|
There was a problem hiding this comment. Import of 'PILImageType' is not used.
Suggested change
|
||||||
| else: | ||||||
| PILImageType = Any | ||||||
|
|
||||||
| _PNG_EXPORT_WARNING_EMITTED = False | ||||||
|
|
||||||
| _DEFAULT_PLUGIN_CACHE: dict[str, Any] = {} | ||||||
| _logger = logging.getLogger(__name__) | ||||||
|
|
||||||
|
|
||||||
| def _iter_frames(pixel_array: np.ndarray) -> Iterator[tuple[int, np.ndarray, bool]]: | ||||||
| """Yield per-frame arrays and whether they represent color data.""" | ||||||
| arr = np.asarray(pixel_array) | ||||||
| if arr.ndim == 2: | ||||||
| yield 0, arr, False | ||||||
| return | ||||||
|
|
||||||
| if arr.ndim == 3: | ||||||
| if arr.shape[-1] in (3, 4): | ||||||
| yield 0, arr, True | ||||||
| else: | ||||||
| for index in range(arr.shape[0]): | ||||||
| frame = arr[index] | ||||||
| is_color = frame.ndim == 3 and frame.shape[-1] in (3, 4) | ||||||
|
There was a problem hiding this comment.
One sanity check: can there be a multi channel image in pydicom with more that 4 channels? Because here you assumes that only if num channels is 3 or 4, then this is color image, and otherwise the first channel is the for frame indexing, and then there is width and height. But it could be as well width, height and many channels. Or such mutlichannel images are not allowed? |
||||||
| yield index, frame, is_color | ||||||
| return | ||||||
|
|
||||||
| if arr.ndim == 4: | ||||||
| for index in range(arr.shape[0]): | ||||||
| frame = arr[index] | ||||||
| is_color = frame.ndim == 3 and frame.shape[-1] in (3, 4) | ||||||
|
There was a problem hiding this comment. Similar to my comment above, |
||||||
| yield index, frame, is_color | ||||||
| return | ||||||
|
|
||||||
| raise ValueError(f"Unsupported pixel array shape {arr.shape!r} for PNG export") | ||||||
|
|
||||||
|
|
||||||
| def _prepare_frame_for_png(frame: np.ndarray, is_color: bool) -> np.ndarray: | ||||||
|
Check failure on line 62 in tests/unit/test_decoder_nvimgcodec.py
|
||||||
| """Convert a decoded frame into a dtype supported by PNG writers.""" | ||||||
| arr = np.nan_to_num(np.asarray(frame), copy=False) | ||||||
|
|
||||||
| # Remove singleton channel dimension for grayscale data. | ||||||
| if not is_color and arr.ndim == 3 and arr.shape[-1] == 1: | ||||||
| arr = arr[..., 0] | ||||||
|
|
||||||
| if is_color: | ||||||
| if arr.dtype == np.uint8: | ||||||
| return arr | ||||||
| arr_float = arr.astype(np.float64, copy=False) | ||||||
| arr_min = float(arr_float.min()) | ||||||
| arr_max = float(arr_float.max()) | ||||||
| if arr_max == arr_min: | ||||||
| return np.zeros_like(arr, dtype=np.uint8) | ||||||
| scaled = (arr_float - arr_min) / (arr_max - arr_min) | ||||||
| return np.clip(np.round(scaled * 255.0), 0, 255).astype(np.uint8) | ||||||
|
|
||||||
| # Grayscale path | ||||||
| arr_min = float(arr.min()) | ||||||
| arr_max = float(arr.max()) | ||||||
|
|
||||||
| if np.issubdtype(arr.dtype, np.integer): | ||||||
| if arr_min >= 0 and arr_max <= 255: | ||||||
| return arr.astype(np.uint8, copy=False) | ||||||
| if arr_min >= 0 and arr_max <= 65535: | ||||||
| return arr.astype(np.uint16, copy=False) | ||||||
|
|
||||||
| arr_float = arr.astype(np.float64, copy=False) | ||||||
| arr_min = float(arr_float.min()) | ||||||
| arr_max = float(arr_float.max()) | ||||||
|
Comment on lines
+92
to
+93
There was a problem hiding this comment. In the grayscale path,
Suggested change
|
||||||
| if arr_max == arr_min: | ||||||
| return np.zeros_like(arr_float, dtype=np.uint8) | ||||||
|
|
||||||
| use_uint16 = arr_max - arr_min > 255.0 | ||||||
| scale = 65535.0 if use_uint16 else 255.0 | ||||||
| scaled = (arr_float - arr_min) / (arr_max - arr_min) | ||||||
| scaled = np.clip(np.round(scaled * scale), 0, scale) | ||||||
| target_dtype = np.uint16 if use_uint16 else np.uint8 | ||||||
| return scaled.astype(target_dtype) | ||||||
|
|
||||||
|
|
||||||
| def _save_frames_as_png(pixel_array: np.ndarray, output_dir: Path, file_stem: str) -> None: | ||||||
| """Persist each frame as a PNG image in the specified directory.""" | ||||||
| global _PNG_EXPORT_WARNING_EMITTED | ||||||
|
|
||||||
| if PILImage is None: | ||||||
| if not _PNG_EXPORT_WARNING_EMITTED: | ||||||
| _logger.info("Skipping PNG export because Pillow is not installed.") | ||||||
| _PNG_EXPORT_WARNING_EMITTED = True | ||||||
| return | ||||||
|
|
||||||
| output_dir.mkdir(parents=True, exist_ok=True) | ||||||
| pil_image_cls = cast(Any, PILImage) | ||||||
|
|
||||||
| for frame_index, frame, is_color in _iter_frames(pixel_array): | ||||||
| frame_for_png = _prepare_frame_for_png(frame, is_color) | ||||||
| image = pil_image_cls.fromarray(frame_for_png) | ||||||
| filename = output_dir / f"{file_stem}_frame_{frame_index:04d}.png" | ||||||
| image.save(filename) | ||||||
|
|
||||||
|
|
||||||
| def get_test_dicoms(folder_path: str | None = None): | ||||||
| """Use pydicom package's embedded test DICOM files for testing or a custom folder of DICOM files.""" | ||||||
|
|
@@ -72,6 +179,8 @@ | |||||
| default_decoder_error_message = f"{e}" | ||||||
| default_decoder_errored = True | ||||||
|
|
||||||
| # Remove and cache the other default decoder plugins first | ||||||
| _remove_default_plugins() | ||||||
| # Register the nvimgcodec decoder plugin and unregister it after each use. | ||||||
| register_as_decoder_plugin() | ||||||
| try: | ||||||
|
|
@@ -82,31 +191,42 @@ | |||||
| nvimgcodec_decoder_errored = True | ||||||
| finally: | ||||||
| unregister_as_decoder_plugin() | ||||||
| _restore_default_plugins() | ||||||
|
|
||||||
| if default_decoder_errored and nvimgcodec_decoder_errored: | ||||||
| _logger.info( | ||||||
| f"All decoders encountered errors for transfer syntax {transfer_syntax} in {Path(path).name}:\n" | ||||||
| f"Default decoder error: {default_decoder_error_message}\n" | ||||||
| f"nvimgcodec decoder error: {nvimgcodec_decoder_error_message}" | ||||||
| ) | ||||||
| return | ||||||
| elif nvimgcodec_decoder_errored and not default_decoder_errored: | ||||||
| raise AssertionError( | ||||||
| f"nvimgcodec decoder errored but default decoder succeeded for transfer syntax {transfer_syntax}" | ||||||
| ) | ||||||
|
|
||||||
| assert baseline_pixels.shape == nv_pixels.shape, f"Shape mismatch with transfer syntax {transfer_syntax}" | ||||||
| assert baseline_pixels.dtype == nv_pixels.dtype, f"Dtype mismatch with transfer syntax {transfer_syntax}" | ||||||
| try: | ||||||
| np.testing.assert_allclose(baseline_pixels, nv_pixels, rtol=rtol, atol=atol) | ||||||
| except AssertionError as e: | ||||||
| raise AssertionError(f"Pixels values mismatch with transfer syntax {transfer_syntax}") from e | ||||||
|
|
||||||
|
|
||||||
| def performance_test_nvimgcodec_decoder_against_defaults( | ||||||
| folder_path: str | None = None, png_output_dir: str | None = None | ||||||
| ) -> None: | ||||||
| """Test and compare the performance of the nvimgcodec decoder against the default decoders | ||||||
| with all DICOM files of supported transfer syntaxes in a custom folder or pydicom embedded dataset. | ||||||
|
|
||||||
| If `png_output_dir` is provided, decoded frames are saved as PNG files for both decoders.""" | ||||||
|
|
||||||
| total_baseline_time = 0.0 | ||||||
| total_nvimgcodec_time = 0.0 | ||||||
|
|
||||||
| files_tested_with_perf: dict[str, dict[str, Any]] = {} # key: path, value: performance_metrics | ||||||
| files_with_errors = [] | ||||||
| png_root = Path(png_output_dir).expanduser() if png_output_dir else None | ||||||
|
|
||||||
| try: | ||||||
| unregister_as_decoder_plugin() # Make sure nvimgcodec decoder plugin is not registered | ||||||
|
|
@@ -118,32 +238,44 @@ | |||||
| ds_default = dcmread(path) | ||||||
| transfer_syntax = ds_default.file_meta.TransferSyntaxUID | ||||||
| start = time.perf_counter() | ||||||
| baseline_pixels = ds_default.pixel_array | ||||||
| baseline_execution_time = time.perf_counter() - start | ||||||
| total_baseline_time += baseline_execution_time | ||||||
|
|
||||||
| perf: dict[str, Any] = {} | ||||||
| perf["transfer_syntax"] = transfer_syntax | ||||||
| perf["baseline_execution_time"] = baseline_execution_time | ||||||
| files_tested_with_perf[path] = perf | ||||||
|
|
||||||
| if png_root is not None: | ||||||
| baseline_dir = png_root / Path(path).stem / "default" | ||||||
| _save_frames_as_png(baseline_pixels, baseline_dir, Path(path).stem) | ||||||
| except Exception: | ||||||
| files_with_errors.append(Path(path).name) | ||||||
| continue | ||||||
|
|
||||||
| _remove_default_plugins() | ||||||
| # Register the nvimgcodec decoder plugin and unregister it after each use. | ||||||
| register_as_decoder_plugin() | ||||||
|
|
||||||
| combined_perf = {} | ||||||
| for path, perf in files_tested_with_perf.items(): | ||||||
| try: | ||||||
| ds_custom = dcmread(path) | ||||||
| start = time.perf_counter() | ||||||
| nv_pixels = ds_custom.pixel_array | ||||||
| perf["nvimgcodec_execution_time"] = time.perf_counter() - start | ||||||
| total_nvimgcodec_time += perf["nvimgcodec_execution_time"] | ||||||
| combined_perf[path] = perf | ||||||
|
|
||||||
| if png_root is not None: | ||||||
| nv_dir = png_root / Path(path).stem / "nvimgcodec" | ||||||
| _save_frames_as_png(nv_pixels, nv_dir, Path(path).stem) | ||||||
| except Exception as e: | ||||||
| _logger.info(f"Error decoding {path} with nvimgcodec decoder: {e}") | ||||||
| continue | ||||||
| unregister_as_decoder_plugin() | ||||||
| _restore_default_plugins() | ||||||
|
|
||||||
| # Performance of the nvimgcodec decoder against the default decoders | ||||||
| # with all DICOM files of supported transfer syntaxes | ||||||
|
|
@@ -167,11 +299,33 @@ | |||||
| print(f"\n\n__Files not tested due to errors encountered by default decoders__: \n{files_with_errors}") | ||||||
|
|
||||||
|
|
||||||
| def _remove_default_plugins(): | ||||||
| """Remove the default plugins from the supported decoder classes.""" | ||||||
|
|
||||||
| global _DEFAULT_PLUGIN_CACHE | ||||||
| for decoder_class in SUPPORTED_DECODER_CLASSES: | ||||||
| _DEFAULT_PLUGIN_CACHE[decoder_class.UID.name] = ( | ||||||
| decoder_class._available | ||||||
| ) # while box, no API to get DecodeFunction | ||||||
| _logger.info(f"Removing default plugins of {decoder_class}: {decoder_class.available_plugins}.") | ||||||
| decoder_class._available = {} # remove all plugins, ref is still held by _DEFAULT_PLUGIN_CACHE | ||||||
| _logger.info(f"Removed default plugins of {decoder_class}: {decoder_class.available_plugins}.") | ||||||
|
|
||||||
|
|
||||||
| def _restore_default_plugins(): | ||||||
| """Restore the default plugins to the supported decoder classes.""" | ||||||
| for decoder_class in SUPPORTED_DECODER_CLASSES: | ||||||
| decoder_class._available = _DEFAULT_PLUGIN_CACHE[decoder_class.UID.name] # restore all plugins | ||||||
| _logger.info(f"Restored default plugins of {decoder_class}: {decoder_class.available_plugins}.") | ||||||
|
|
||||||
|
|
||||||
| if __name__ == "__main__": | ||||||
|
|
||||||
| # Use pytest to test the functionality with pydicom embedded DICOM files of supported transfer syntaxes individually | ||||||
| # python -m pytest test_decoder_nvimgcodec.py | ||||||
| # | ||||||
| # The following compares the performance of the nvimgcodec decoder against the default decoders | ||||||
| # with DICOM files in pydicom embedded dataset or an optional custom folder | ||||||
| performance_test_nvimgcodec_decoder_against_defaults( | ||||||
| png_output_dir="decoded_png" | ||||||
| ) # or use ("/tmp/multi-frame-dcm", png_output_dir="decoded_png") | ||||||
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.
Uh oh!
There was an error while loading. Please reload this page.