Project-MONAI / MONAI

AI Toolkit for Healthcare Imaging
https://monai.io/
Apache License 2.0
5.85k stars 1.08k forks source link

consistent usages of `np.RandomState` in the code #6888

Open wyli opened 1 year ago

wyli commented 1 year ago
  1. There are some usages of np.random.* in the transforms (see below). ..

./monai/transforms/signal/array.py:276:np.random.choice ./monai/transforms/signal/array.py:357:np.random.choice ./monai/transforms/spatial/array.py:1214:np.random.randint ./monai/transforms/spatial/dictionary.py:2347:np.random.randint ./monai/transforms/spatial/dictionary.py:698:np.random.randint ./monai/transforms/utils.py:1220:np.random.random ./monai/transforms/utils.py:246:np.random.randint ./monai/transforms/utils.py:510:np.random.randint ./monai/transforms/utils.py:552:np.random.random ./monai/transforms/utils.py:607:np.random.random

  1. There are some usages in other parts of the main code. Same question on what to do with those remains:

./monai/apps/deepedit/interaction.py:65:np.random.choice ./monai/apps/deepedit/transforms.py:159:np.random.choice ./monai/apps/deepedit/transforms.py:60:np.random.choice ./monai/apps/detection/transforms/dictionary.py:1305:np.random.randint ./monai/apps/nuclick/transforms.py:370:np.random.choice ./monai/apps/nuclick/transforms.py:377:np.random.choice ./monai/auto3dseg/analyzer.py:190:np.random.rand ./monai/auto3dseg/analyzer.py:191:np.random.rand ./monai/auto3dseg/analyzer.py:292:np.random.rand ./monai/auto3dseg/analyzer.py:374:np.random.rand ./monai/auto3dseg/analyzer.py:862:np.random.rand ./monai/auto3dseg/operations.py:119:np.random.rand ./monai/auto3dseg/operations.py:120:np.random.rand ./monai/auto3dseg/operations.py:121:np.random.rand ./monai/auto3dseg/operations.py:122:np.random.rand ./monai/auto3dseg/operations.py:62:np.random.rand ./monai/data/synthetic.py:142:np.random.random ./monai/data/synthetic.py:65:np.random.random ./monai/data/utils.py:125:np.random.randint ./monai/utils/misc.py:353:np.random.seed ./monai/visualize/utils.py:92:np.random.rand ./monai/visualize/utils.py:97:np.random.rand

  1. What to do with usage of np.random.* in tests/*/.py files? Numpy states that:

New code should use the normal method of a Generator instance instead; please see the Quick Start.

So it seems like there is no need to migrate those immediately, but just wanted to mention that here. Since there are around ~390 usages in tests I am not listing them here.

I found usages using the following bash command: find . -name "*.py" -exec grep -o -H -n -E 'np.random.\w+' {} \; | grep -v "RandomState" | sort | uniq

Originally posted by @john-zielke-snkeos in https://github.com/Project-MONAI/MONAI/issues/6854#issuecomment-1682375125

wyli commented 1 year ago

(cc @ericspod @john-zielke-snkeos follow-up of the discussions in #6854, self.R and RandomState is currently preferred over np.random.*, this ticket is to track the issue. e.g. preferred usage: https://github.com/Project-MONAI/MONAI/blob/8aabdc917bbf059800968906af22c62b79a6c763/monai/data/synthetic.py#L65)

ericspod commented 1 year ago

If we wanted to first change the usages of np.random.* to use a RandomState then migrate in the future to Generator we could do the following:

We do need to have a more considered discussion about what to do about migration at some point, either we move to Generator or we do implement our own class to replace RandomState usage.