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.

Sign up for GitHub

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

Jump to bottom

OverflowError in transform.py Randomizable set_random_state() with seed #7871

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

OverflowError in transform.py Randomizable set_random_state() with seed #7871

wants to merge 1 commit into from

Conversation

GenevieveBuckley
Copy link

Fixes # teamtomo/membrain-seg#70

Description

I'm using the membrain_seg package, which depends on MONAI, and have found that it consistently triggers an overflow error in MONAI

I've been able to fix the problem by ensuring both _seed and MAX_SEED are 64 bit integers, like this:

_seed = int(_seed) % MAX_SEED

and this has fixed the problem for me.

I notice that when the OverflowError occurs, _seed is a numpy uint32 bit value. I suspect this is the source of the problem, since MAX_SEED is by definition int64

MONAI/monai/utils/misc.py

Lines 84 to 85 in 08d5728

NP_MAX = np.iinfo(np.uint32).max
MAX_SEED = NP_MAX + 1 # 2**32, the actual seed should be in [0, MAX_SEED - 1] for uint32

For reference, here is a copy of the original bug report

  • membrain-seg version: current git main branch
  • Python version: tested with python 3.9 and python 3.11
  • Operating System: Zorin OS 16.3

Description

I followed the instructions in the installation guide, and all the packages appeared to install correctly with pip. But, when I tried to launch membrain from the command line, I got an error.

What I Did

git clone https://github.com/teamtomo/membrain-seg.git
cd membrain_seg
conda create -n membrainseg python=3.9 pip
conda activate membrainseg
pip install .
membrain

$ membrain

Traceback (most recent call last):
  File "/home/supervisor/miniforge3/envs/membrainseg/bin/membrain", line 5, in <module>
    from membrain_seg.segmentation.cli import cli
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/membrain_seg/segmentation/cli/__init__.py", line 5, in <module>
    from .segment_cli import segment  # noqa: F401
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/membrain_seg/segmentation/cli/segment_cli.py", line 12, in <module>
    from ..segment import segment as _segment
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/membrain_seg/segmentation/segment.py", line 17, in <module>
    from .dataloading.memseg_augmentation import get_mirrored_img, get_prediction_transforms
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/membrain_seg/segmentation/dataloading/memseg_augmentation.py", line 18, in <module>
    from membrain_seg.segmentation.dataloading.transforms import (
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/membrain_seg/segmentation/dataloading/transforms.py", line 608, in <module>
    AxesShuffle = Compose(
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/monai/transforms/compose.py", line 251, in __init__
    self.set_random_state(seed=get_seed())
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/monai/transforms/compose.py", line 263, in set_random_state
    _transform.set_random_state(seed=self.R.randint(MAX_SEED, dtype="uint32"))
  File "/home/supervisor/miniforge3/envs/membrainseg/lib/python3.9/site-packages/monai/transforms/transform.py", line 207, in set_random_state
    _seed = _seed % MAX_SEED
OverflowError: Python integer 4294967296 out of bounds for uint32

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • 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.

@GenevieveBuckley
Copy link
Author

I'd like to add a test for this, however I'm having trouble reducing it to a minimal reproducible example, without involving the external membrain_seg package.

Here's where I think the extra test case should go:

MONAI/tests/test_randomizable.py

Lines 37 to 43 in 08d5728

def test_seed(self):
inst = RandTest()
inst.set_random_state(seed=123)
self.assertAlmostEqual(inst.R.rand(), 0.69646918)
inst.set_random_state(123)
self.assertAlmostEqual(inst.R.rand(), 0.69646918)

What I've tried so far is hitting the error, entering the debugger, and then copy-pasting the exact value of _seed at that point into a new line in test_seed, eg: inst.set_random_state(seed=np.uint32(1126710810))
... but running just this test case doesn't produce the same overflow error as before. I don't know if there are other types of hidden state complicating things.

Do you have any suggestions?

@GenevieveBuckley GenevieveBuckley mentioned this pull request Jun 24, 2024
Closed
@ericspod
Copy link
Member

Hi @GenevieveBuckley This is cause by our incompatibility with Numpy 2.0. If you install a 1.* version this will go away. We're working on a fix now as a stopgap. I also had a previous PR on the same subject but if you wanted to work on other fixes we'll need these eventually. Thanks!

@clead6
Copy link

clead6 commented Dec 3, 2024

Hi - I'm currently running into a similar error when using MONAI transforms with a random seed. Like you said downgrading numpy to version 1.* fixed it, but it would be great if this could be patched!

@ericspod
Copy link
Member

ericspod commented Dec 3, 2024

Hi - I'm currently running into a similar error when using MONAI transforms with a random seed. Like you said downgrading numpy to version 1.* fixed it, but it would be great if this could be patched!

Are you using the most current version of MONAI? We did add fixes that should work with Numpy 2.0, although we've left the official requirement as Numpy 1.x. This should be accounted for now here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@GenevieveBuckley @ericspod @clead6