Skip to content
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

Faulty imports of packaging in version comparison logic #8349

Closed
nkaenzig opened this issue Feb 18, 2025 · 0 comments · Fixed by #8347
Closed

Faulty imports of packaging in version comparison logic #8349

nkaenzig opened this issue Feb 18, 2025 · 0 comments · Fixed by #8347
Labels
bug Something isn't working

Comments

@nkaenzig
Copy link
Contributor

nkaenzig commented Feb 18, 2025

Describe the bug
Wrong import statements in the functions version_leq and version_geq within monai.utils.module:

pkging, has_ver = optional_import("packaging.Version")

pkging, has_ver = optional_import("packaging.Version")

The Version class is exposed by the packaging.version submodule, therefore pkging, has_ver = optional_import("packaging.Version") always returns has_ver=False.

To Reproduce
Executing import packaging.Version in a python shell or script yields this error:

ModuleNotFoundError: No module named 'packaging.Version'

This issue currently doesn't surface because version_leq and version_geq both implement a fallback logic which is used when the import of packaging fails. However, I've observed more hidden consequences of this as described in #8348.

Expected behavior
Use packaging.version.Version for version comparisons, not the fallback after a failed import.

Environment

Ensuring you use the relevant python executable, please paste the output of:

================================
Printing MONAI config...
================================
MONAI version: 1.4.0
Numpy version: 1.26.4
Pytorch version: 2.5.1
MONAI flags: HAS_EXT = False, USE_COMPILED = False, USE_META_DICT = False
MONAI rev id: 46a5272196a6c2590ca2589029eed8e4d56ff008
MONAI __file__: /Users/<username>/workspace/kaiko-eng-worktrees/kaiko-eng/dist/export/python/virtualenvs/python-default/3.11.10/lib/python3.11/site-packages/monai/__init__.py

Optional dependencies:
Pytorch Ignite version: NOT INSTALLED or UNKNOWN VERSION.
ITK version: NOT INSTALLED or UNKNOWN VERSION.
Nibabel version: 4.0.2
scikit-image version: 0.24.0
scipy version: 1.14.1
Pillow version: 10.4.0
Tensorboard version: 2.18.0
gdown version: 5.2.0
TorchVision version: 0.20.1
tqdm version: 4.66.6
lmdb version: NOT INSTALLED or UNKNOWN VERSION.
psutil version: 6.1.0
pandas version: 2.1.4
einops version: 0.8.0
transformers version: 4.48.1
mlflow version: NOT INSTALLED or UNKNOWN VERSION.
pynrrd version: NOT INSTALLED or UNKNOWN VERSION.
clearml version: NOT INSTALLED or UNKNOWN VERSION.

For details about installing the optional dependencies, please visit:
    https://docs.monai.io/en/latest/installation.html#installing-the-recommended-dependencies


================================
Printing system config...
================================
System: Darwin
Mac version: 15.3.1
Platform: macOS-15.3.1-arm64-arm-64bit
Processor: arm
Machine: arm64
Python version: 3.11.10
Process name: python3.11
Command: ['python', '-c', 'import monai; monai.config.print_debug_info()']
Open files: []
Num physical CPUs: 10
Num logical CPUs: 10
Num usable CPUs: UNKNOWN for given OS
CPU usage (%): [47.4, 44.1, 61.7, 49.2, 40.5, 25.0, 26.1, 13.3, 8.1, 7.2]
CPU freq. (MHz): 3228
Load avg. in last 1, 5, 15 mins (%): [29.6, 26.7, 32.7]
Disk usage (%): 76.4
Avg. sensor temp. (Celsius): UNKNOWN for given OS
Total physical memory (GB): 64.0
Available memory (GB): 49.6
Used memory (GB): 12.3

================================
Printing GPU config...
================================
Num GPUs: 0
Has CUDA: False
cuDNN enabled: False
NVIDIA_TF32_OVERRIDE: None
TORCH_ALLOW_TF32_CUBLAS_OVERRIDE: None```
@KumoLiu KumoLiu added the bug Something isn't working label Feb 19, 2025
Can-Zhao pushed a commit to Can-Zhao/MONAI that referenced this issue Mar 10, 2025
Fixes Project-MONAI#8349

### Description

The current behaviour is that `pkging, has_ver =
optional_import("packaging.Version")` always returns `has_ver=False`
because the import always fails (the `Version` class is exposed by the
`packaging.version` submodule).

This issue previously didn't surface, because when the import fails, it
would just continue to use the fallback logic. However, there seem to be
more hidden and more severe implications, which ultimately led me to
discovering this particular bug: Function like `floor_divide()` in
`monai.transforms` that check the module version using this logic are
called many times in common ML dataloading workflows. The failed imports
somehow can lead to OOM errors and the main process being killed (see
Project-MONAI#8348). Maybe when
`optional_import` fails to import a module, the lazy exceptions somehow
stack up in memory when this function is called many times in a short
time period?

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [ ] New tests added to cover the changes.
- [ ] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [ ] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [ ] In-line docstrings updated.
- [ ] Documentation updated, tested `make html` command in the `docs/`
folder.

Signed-off-by: Can-Zhao <volcanofly@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants