broken nnunetv2 import

[jeandre-db]

ImportError: cannot import name 'crop_to_bbox' from 'acvl_utils.cropping_and_padding.bounding_boxes' (/local_disk0/.ephemeral_nfs/cluster_libraries/python/lib/python3.11/site-packages/acvl_utils/cropping_and_padding/bounding_boxes.py)
File , line 7
5 import pyspark.sql.functions as f
6 from nnunetv2.imageio.simpleitk_reader_writer import SimpleITKIO
----> 7 from nnunetv2.inference.predict_from_raw_data import nnUNetPredictor
8 from nnunetv2.paths import nnUNet_results, nnUNet_raw
9 import torch

It seems that the update to acvl broke nnunetv2 imports. locking the version to 0.2 fixes this issue.

[joshuacwnewton]

I'm getting the same error. Here's a more detailed traceback:

  File "/home/runner/sct_0.0/spinalcordtoolbox/deepseg/inference.py", line 27, in <module>
    import spinalcordtoolbox.deepseg.nnunet as ds_nnunet
  File "/home/runner/sct_0.0/spinalcordtoolbox/deepseg/nnunet.py", line 13, in <module>
    from nnunetv2.inference.predict_from_raw_data import nnUNetPredictor   # noqa: E402
  File "/home/runner/sct_0.0/python/envs/venv_sct/lib/python3.9/site-packages/nnunetv2/inference/predict_from_raw_data.py", line 22, in <module>
    from nnunetv2.inference.data_iterators import PreprocessAdapterFromNpy, preprocessing_iterator_fromfiles, \
  File "/home/runner/sct_0.0/python/envs/venv_sct/lib/python3.9/site-packages/nnunetv2/inference/data_iterators.py", line 12, in <module>
    from nnunetv2.preprocessing.preprocessors.default_preprocessor import DefaultPreprocessor
  File "/home/runner/sct_0.0/python/envs/venv_sct/lib/python3.9/site-packages/nnunetv2/preprocessing/preprocessors/default_preprocessor.py", line 23, in <module>
    from nnunetv2.preprocessing.cropping.cropping import crop_to_nonzero
  File "/home/runner/sct_0.0/python/envs/venv_sct/lib/python3.9/site-packages/nnunetv2/preprocessing/cropping/cropping.py", line 5, in <module>
    from acvl_utils.cropping_and_padding.bounding_boxes import get_bbox_from_mask, crop_to_bbox, bounding_box_to_slice
  File "/home/runner/sct_0.0/python/envs/venv_sct/lib/python3.9/site-packages/acvl_utils/cropping_and_padding/bounding_boxes.py", line 5, in <module>
    import blosc2
ModuleNotFoundError: No module named 'blosc2'

The culprit seems to be e49f45c, where an import blosc2 was added. I presume that package is present in @FabianIsensee's development environment, but missing in acvl_utils's dependencies:

acvl_utils/setup.py

Lines 11 to 18 in b2493d4

	install_requires=[
	"numpy",
	"batchgenerators",
	"torch",
	"SimpleITK",
	"scikit-image",
	"connected-components-3d"
	],

I assume the fix should be as simple as pip install blosc2 (or maybe changing the import to an optional import), but I imagine that acvs_utils==0.2.1 should be yanked as well?

[jeandre-db]

even with the import blosc2, the crop_to_bbox error persists unless you use acvl_utils==0.2

[alireza-hokmabadi]

I had the same error and found that the crop_to_bbox function from acvl_utils/cropping_and_padding/bounding_boxes.py was recently removed, which is causing this import issue with nnUNetPredictor.

By adding this function back into the file, the problem can be resolved. I've made a pull request to address this, and hopefully, it will be merged soon.

Proposed Solution: Add the following function to bounding_boxes.py:

def crop_to_bbox(array: np.ndarray, bounding_box: List[List[int]]): assert len(bounding_box) == len(array.shape), f"Dimensionality of bbox and array do not match. bbox has length " \ f"{len(bounding_box)} while array has dimension {len(array.shape)}" slicer = bounding_box_to_slice(bounding_box) return array[slicer]

[jamesobutler]

This issue has been reported by a user of TotalSegmentator using the 3D Slicer application. See https://discourse.slicer.org/t/totalsegmentator-failed-to-compute-results-returned-non-zero-exit-status-1/38142/6?u=jamesobutler

[lassoan]

Error reports are flooding in for 3D Slicer for TotalSegmentator. It would be great if this issue could be fixed soon! Thank you!

[FabianIsensee]

I am on a trip, will do this tomorrow when I am back. Sorry :-/

[FabianIsensee]

Hey all I tried to patch it from where I am. There is now a v0.2.2 with the missing function added back in and the blosc2 dependency added. I hope this fixes it. Please let me know if it works now.
I will in the future set explicit package version dependencies in each release of nnU-Net (== instead of >=) to ensure this doesn't happen again. Not a big fan of the rigidity this causes but that's better than running into situations like these. Sorry for all the chaos I have caused!
I will also yank v0.2.1 from pypi as soon as I can. I currently don't have access to that
Best,
Fabian

[lassoan]

Thank you!

Fixing the problem and yanking the broken version are sufficient as corrective actions. For preventive action, you could set up continuous testing by github actions (test all features that you would feel embarrassed about if they were broken).

Please do not set version with == as it can cause more problems than it solves (lot of unresolvable version incompatibility issues may come up at application level).

[hhaentze]

nnUNetv2 is working again on my system. Thanks for the patch!

[pwrightkcl]

This error came up with me on rebuilding a Docker image that installs nnunetv2==2.3.1. We need fixed versions because this is a deployed app so the inference version has to match the training version. Can a hotfix be applied to nnunetv2 2.3.1 so it's not broken by the downstream changes?

[jamesobutler]

@pwrightkcl nnunetv2 v2.3.1 allows acvl_utils 0.2.x, so no hotfix is necessary. Just don’t use 0.2.1, you can use 0.2.2 or 0.2.0 and you can install this package directly or through clean install to pull in the latest working 0.2.2.
https://github.com/MIC-DKFZ/nnUNet/blob/1b5a17daedb819b6d0be571598a1384a8a9befc5/pyproject.toml#L34

[lassoan]

We need fixed versions because this is a deployed app so the inference version has to match the training version. Can a hotfix be applied to nnunetv2 2.3.1 so it's not broken by the downstream changes?

In this issue we are talking about version requirements at library level. Python packages should never require exact versions of packages that they rely on (version requirements should be as broad as possible), because otherwise it would be practically impossible to pip-install more than a few Python packages in an environment.

At application level (e.g., in your application's installation script), it can make sense to specify exact version of each and every Python package for reproducibility.

[pwrightkcl]

@lassoan this is a production app deployed in clinical environments based in Docker images, so we need to be able to build reproducibly. In this case, unit tests started failing for the component that uses nnunet despite no changes to the code and all requirements having fixed versions.

My view is that a hotfix is still required to get nnunet 2.3.1 back to working "out of the box" as it used to. If I were in a dev / research environment, it wouldn't be a big deal to add a new line to my requirements file or even install from some custom git commit. But that is not my use case. I think it would also prevent anyone else having this issue from having to track down this issue thread.

[lassoan]

this is a production app deployed in clinical environments based in Docker images, so we need to be able to build reproducibly. In this case, unit tests started failing for the component that uses nnunet despite no changes to the code and all requirements having fixed versions.

It sounds like you have only specified some top-level version requirements. You actually need to freeze the version of all the packages that you use (recursively, for all dependencies of dependencies). Since only you know what is a working version combination of all the packages that you need, only you can do this, at the application level.

Fortunately, this is very easy to do, as you can run pip freeze to get a list of all installed package versions in your current environment and use it as requirements when setting up your container.

My view is that a hotfix is still required to get nnunet 2.3.1 back to working "out of the box" as it used to.

Of course, any broken Python package version must be yanked from PyPI. In the acvl-util release history it seems that the broken 0.2.1 version has still has not been yanked yet.

@FabianIsensee could you please yank acvl-utils 0.2.1 from PyPI? It would make pip automatically uninstall/skip this broken version in existing and future installs. It is really simple, just log in to pypi, go to https://pypi.org/manage/project/acvl-utils/releases/, click on Options next to 0.2.1 and choose Yank.

[pwrightkcl]

@lassoan I'll look into doing a complete pip freeze for all our images.

For nnunetv2 2.3.1, it still seems to be installing acvl_utils 0.2.1, not 0.2.2.

[jamesobutler]

For nnunetv2 2.3.1, it still seems to be installing acvl_utils 0.2.1, not 0.2.2.

This would only be if you are not installing it fresh (aka nothing in your pip cache). Your pip cache will reuse a downloaded whl file if it still passes the version range checks. At this side acvl_utils should yank the release so that is not pulled to download any more, and nnutev2 could specify an exclusion for 0.2.1, but you can handle this from your application side without waiting for those safeguards. You can run a pip install upgrade command to make sure to pull the latest always. But as @lassoan suggested you are looking for a pip freeze type action to install hardened versions of all packages including their dependencies (and dependencies of the direct dependencies). A library is not supposed to do that, but that can make sense from your application level.

As you can review in nnunetv2 v2.3.1 set of dependenices there are no hardened pinned versions. When setting up a docker image fresh you would get a different version of torch if you did it today and a year from now as torch will release new versions.
https://github.com/MIC-DKFZ/nnUNet/blob/1b5a17daedb819b6d0be571598a1384a8a9befc5/pyproject.toml#L32

[pwrightkcl]

@jamesobutler yes, TIL about freezing deeper dependencies (thanks both). I am implementing this as I type. In case anyone in my position finds it useful, I also learned pip freeze includes local file links, but pip list --freeze just lists the pypi modules.

The bit about 0.2.1 being used not 0.2.2 was just FYI, since the above solves it for me. I don't think it's pip cache, as I built a fresh docker image with no caches, but I just tried in a local venv instead and it installed acvl_utils==0.2.2, so I guess disregard.

[lassoan]

pip maintains its own system-wide cache, which is independent from docker cache.

[pwrightkcl]

Interesting. So even inside a Docker build, pip will search the local system cache? I would have expected it to only look inside the docker environment. Getting a bit off-topic, I suppose, since the issue is resolved, but interesting to me.

[jamesobutler]

Of course, any broken Python package version must be yanked from PyPI. In the acvl-util release history it seems that the broken 0.2.1 version has still has not been yanked yet.

@FabianIsensee could you please yank acvl-utils 0.2.1 from PyPI? It would make pip automatically uninstall/skip this broken version in existing and future installs. It is really simple, just log in to pypi, go to https://pypi.org/manage/project/acvl-utils/releases/, click on Options next to 0.2.1 and choose Yank.

@FabianIsensee it appears 0.2.1 is still not yanked on PyPI. Do you have plans to complete this action?

[lassoan]

This issue is still continuously burning users. Due to extensive caching in pip, a having 0.2.2 version is not sufficient, the broken 0.2.1 will still be installed on many users' computers by default, until you yank this corrupted package.

@FabianIsensee please yank acvl-utils 0.2.1. We provide user support for nnUnet for free and we are struggling to keep up with the demand because of this issue.