Skip to content

NumpyBackend.seed() rejects a np.random.RandomState instance, forcing callers to special-case it #848

Description

@Samuel-Vangu

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions