Describe the bug
NumpyBackend.seed() only accepts an int (or None). Passing an
already-instantiated np.random.RandomState raises a TypeError,
because RandomState.seed() itself doesn't accept another RandomState
as input:
TypeError: Cannot cast scalar from dtype('O') to dtype('int64')
according to the rule 'safe'
As a result, every caller that wants to support passing an existing
RandomState has to special-case it manually instead of relying on the
shared nx.seed(seed) / nx.randn(...) abstraction. TorchBackend and
TensorflowBackend already handle this correctly -- both adopt an
externally-passed generator (torch.Generator, tf.random.Generator)
directly. NumpyBackend is the outlier.
Code sample
import numpy as np
from ot.backend import NumpyBackend
nx = NumpyBackend()
rng = np.random.RandomState(42)
nx.seed(rng) # raises TypeError
Expected behavior
nx.seed(rng) should adopt rng as the backend's own generator (same
pattern as TorchBackend/TensorflowBackend), so later nx.randn(...)
calls draw from it, with no error and no special-casing needed by callers.
Fix, in NumpyBackend.seed():
def seed(self, seed=None):
if isinstance(seed, np.random.RandomState):
self.rng_ = seed
elif seed is not None:
self.rng_.seed(seed)
Once this lands, the isinstance(seed, np.random.RandomState) and str(nx) == "numpy" special case can be removed from 4 places in
ot/sliced/_utils.py (get_random_projections, get_projections_sphere,
get_random_rotations, get_projections_spiral), each simplified to
if seed is not None: nx.seed(seed).
Two other occurrences use different logic and should be checked
separately, not assumed to be the same bug: ot/sliced/_spherical_sliced.py:435
(not isinstance(...), inverted condition) and ot/utils.py:702.
Environment
Not environment-specific -- a pure Python/NumPy logic bug, reproducible
on any platform/version with a recent ot.backend.
Additional context
Found while reviewing PR #838. Will open the fix PR referencing this
issue.
Describe the bug
NumpyBackend.seed()only accepts an int (orNone). Passing analready-instantiated
np.random.RandomStateraises aTypeError,because
RandomState.seed()itself doesn't accept anotherRandomStateas input:
As a result, every caller that wants to support passing an existing
RandomStatehas to special-case it manually instead of relying on theshared
nx.seed(seed)/nx.randn(...)abstraction.TorchBackendandTensorflowBackendalready handle this correctly -- both adopt anexternally-passed generator (
torch.Generator,tf.random.Generator)directly.
NumpyBackendis the outlier.Code sample
Expected behavior
nx.seed(rng)should adoptrngas the backend's own generator (samepattern as
TorchBackend/TensorflowBackend), so laternx.randn(...)calls draw from it, with no error and no special-casing needed by callers.
Fix, in
NumpyBackend.seed():Once this lands, the
isinstance(seed, np.random.RandomState) and str(nx) == "numpy"special case can be removed from 4 places inot/sliced/_utils.py(get_random_projections,get_projections_sphere,get_random_rotations,get_projections_spiral), each simplified toif seed is not None: nx.seed(seed).Two other occurrences use different logic and should be checked
separately, not assumed to be the same bug:
ot/sliced/_spherical_sliced.py:435(
not isinstance(...), inverted condition) andot/utils.py:702.Environment
Not environment-specific -- a pure Python/NumPy logic bug, reproducible
on any platform/version with a recent
ot.backend.Additional context
Found while reviewing PR #838. Will open the fix PR referencing this
issue.