From e9fef02cab885bed30c22ca81782820929637b42 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Mon, 4 Aug 2025 14:50:52 -0700 Subject: [PATCH 1/8] fix: crash when provided seed is out-of-bounds --- src/lightning/fabric/utilities/seed.py | 6 +++--- tests/tests_fabric/utilities/test_seed.py | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/lightning/fabric/utilities/seed.py b/src/lightning/fabric/utilities/seed.py index f9c0ddeb86cf0..44e5bdffb24de 100644 --- a/src/lightning/fabric/utilities/seed.py +++ b/src/lightning/fabric/utilities/seed.py @@ -27,7 +27,8 @@ def seed_everything(seed: Optional[int] = None, workers: bool = False, verbose: Args: seed: the integer value seed for global random state in Lightning. If ``None``, it will read the seed from ``PL_GLOBAL_SEED`` env variable. If ``None`` and the - ``PL_GLOBAL_SEED`` env variable is not set, then the seed defaults to 0. + ``PL_GLOBAL_SEED`` env variable is not set, then the seed defaults to 0. If the seed is provided + as an integer but is not in bounds, a ValueError is raised. workers: if set to ``True``, will properly configure all dataloaders passed to the Trainer with a ``worker_init_fn``. If the user already provides such a function for their dataloaders, setting this argument will have no influence. See also: @@ -50,8 +51,7 @@ def seed_everything(seed: Optional[int] = None, workers: bool = False, verbose: seed = int(seed) if not (min_seed_value <= seed <= max_seed_value): - rank_zero_warn(f"{seed} is not in bounds, numpy accepts from {min_seed_value} to {max_seed_value}") - seed = 0 + raise ValueError(f"{seed} is not in bounds, numpy accepts from {min_seed_value} to {max_seed_value}") if verbose: log.info(rank_prefixed_message(f"Seed set to {seed}", _get_rank())) diff --git a/tests/tests_fabric/utilities/test_seed.py b/tests/tests_fabric/utilities/test_seed.py index 4a948a5f98736..7578c0814cd12 100644 --- a/tests/tests_fabric/utilities/test_seed.py +++ b/tests/tests_fabric/utilities/test_seed.py @@ -56,10 +56,9 @@ def test_invalid_seed(): @mock.patch.dict(os.environ, {}, clear=True) @pytest.mark.parametrize("seed", [10e9, -10e9]) def test_out_of_bounds_seed(seed): - """Ensure that we still fix the seed even if an out-of-bounds seed is given.""" - with pytest.warns(UserWarning, match="is not in bounds"): - actual = seed_everything(seed) - assert actual == 0 + """Ensure that a ValueError is raised if an out-of-bounds seed is given.""" + with pytest.raises(ValueError, match="is not in bounds"): + seed_everything(seed) def test_reset_seed_no_op(): From 34302e23f6596c0800368db510c1aa230d0bc194 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Mon, 4 Aug 2025 15:19:16 -0700 Subject: [PATCH 2/8] fix: update handling of invalid seeds via PL_GLOBAL_SEED --- src/lightning/fabric/utilities/seed.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lightning/fabric/utilities/seed.py b/src/lightning/fabric/utilities/seed.py index 44e5bdffb24de..28bb9c1e90111 100644 --- a/src/lightning/fabric/utilities/seed.py +++ b/src/lightning/fabric/utilities/seed.py @@ -28,7 +28,7 @@ def seed_everything(seed: Optional[int] = None, workers: bool = False, verbose: seed: the integer value seed for global random state in Lightning. If ``None``, it will read the seed from ``PL_GLOBAL_SEED`` env variable. If ``None`` and the ``PL_GLOBAL_SEED`` env variable is not set, then the seed defaults to 0. If the seed is provided - as an integer but is not in bounds, a ValueError is raised. + but is not in bounds or cannot be casted to int, a ValueError is raised. workers: if set to ``True``, will properly configure all dataloaders passed to the Trainer with a ``worker_init_fn``. If the user already provides such a function for their dataloaders, setting this argument will have no influence. See also: @@ -45,8 +45,7 @@ def seed_everything(seed: Optional[int] = None, workers: bool = False, verbose: try: seed = int(env_seed) except ValueError: - seed = 0 - rank_zero_warn(f"Invalid seed found: {repr(env_seed)}, seed set to {seed}") + raise ValueError(f"Invalid seed specified via PL_GLOBAL_SEED: {repr(env_seed)}") elif not isinstance(seed, int): seed = int(seed) From 86c26d89a2cd90056dacea18da85f85aece80f30 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Mon, 4 Aug 2025 15:21:16 -0700 Subject: [PATCH 3/8] fix: update test for invalid env var seed --- tests/tests_fabric/utilities/test_seed.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/tests_fabric/utilities/test_seed.py b/tests/tests_fabric/utilities/test_seed.py index 7578c0814cd12..e0ef478ff9cab 100644 --- a/tests/tests_fabric/utilities/test_seed.py +++ b/tests/tests_fabric/utilities/test_seed.py @@ -47,11 +47,9 @@ def test_correct_seed_with_environment_variable(): @mock.patch.dict(os.environ, {"PL_GLOBAL_SEED": "invalid"}, clear=True) def test_invalid_seed(): - """Ensure that we still fix the seed even if an invalid seed is given.""" - with pytest.warns(UserWarning, match="Invalid seed found"): + """Ensure that a ValueError is raised if an invalid seed is given.""" + with pytest.raises(ValueError, match="Invalid seed specified"): seed = seed_everything() - assert seed == 0 - @mock.patch.dict(os.environ, {}, clear=True) @pytest.mark.parametrize("seed", [10e9, -10e9]) From 08dde6651490d0efe679ef4f0dd2fad4919ad7c2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 4 Aug 2025 22:21:33 +0000 Subject: [PATCH 4/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/tests_fabric/utilities/test_seed.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/tests_fabric/utilities/test_seed.py b/tests/tests_fabric/utilities/test_seed.py index e0ef478ff9cab..ecd53fa4521be 100644 --- a/tests/tests_fabric/utilities/test_seed.py +++ b/tests/tests_fabric/utilities/test_seed.py @@ -49,7 +49,8 @@ def test_correct_seed_with_environment_variable(): def test_invalid_seed(): """Ensure that a ValueError is raised if an invalid seed is given.""" with pytest.raises(ValueError, match="Invalid seed specified"): - seed = seed_everything() + seed_everything() + @mock.patch.dict(os.environ, {}, clear=True) @pytest.mark.parametrize("seed", [10e9, -10e9]) From 0418a82d145a50057681a4ed436a3f9c9ad21b1c Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Mon, 4 Aug 2025 15:22:53 -0700 Subject: [PATCH 5/8] fix: typo --- src/lightning/fabric/utilities/seed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lightning/fabric/utilities/seed.py b/src/lightning/fabric/utilities/seed.py index 28bb9c1e90111..5ab86e7498c13 100644 --- a/src/lightning/fabric/utilities/seed.py +++ b/src/lightning/fabric/utilities/seed.py @@ -28,7 +28,7 @@ def seed_everything(seed: Optional[int] = None, workers: bool = False, verbose: seed: the integer value seed for global random state in Lightning. If ``None``, it will read the seed from ``PL_GLOBAL_SEED`` env variable. If ``None`` and the ``PL_GLOBAL_SEED`` env variable is not set, then the seed defaults to 0. If the seed is provided - but is not in bounds or cannot be casted to int, a ValueError is raised. + but is not in bounds or cannot be cast to int, a ValueError is raised. workers: if set to ``True``, will properly configure all dataloaders passed to the Trainer with a ``worker_init_fn``. If the user already provides such a function for their dataloaders, setting this argument will have no influence. See also: From e2b322502ce9ad7b1a051e00713c074fde665b33 Mon Sep 17 00:00:00 2001 From: Jonathan King Date: Mon, 4 Aug 2025 15:28:24 -0700 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Deependu --- src/lightning/fabric/utilities/seed.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lightning/fabric/utilities/seed.py b/src/lightning/fabric/utilities/seed.py index 5ab86e7498c13..534e5e3db653e 100644 --- a/src/lightning/fabric/utilities/seed.py +++ b/src/lightning/fabric/utilities/seed.py @@ -27,8 +27,8 @@ def seed_everything(seed: Optional[int] = None, workers: bool = False, verbose: Args: seed: the integer value seed for global random state in Lightning. If ``None``, it will read the seed from ``PL_GLOBAL_SEED`` env variable. If ``None`` and the - ``PL_GLOBAL_SEED`` env variable is not set, then the seed defaults to 0. If the seed is provided - but is not in bounds or cannot be cast to int, a ValueError is raised. + ``PL_GLOBAL_SEED`` env variable is not set, then the seed defaults to 0. If seed is + not in bounds or cannot be cast to int, a ValueError is raised. workers: if set to ``True``, will properly configure all dataloaders passed to the Trainer with a ``worker_init_fn``. If the user already provides such a function for their dataloaders, setting this argument will have no influence. See also: From b928bf1a002ce938dd4a9cdf2625aa0910e76aec Mon Sep 17 00:00:00 2001 From: Deependu Jha Date: Tue, 5 Aug 2025 04:18:03 +0530 Subject: [PATCH 7/8] changelog --- src/lightning/fabric/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lightning/fabric/CHANGELOG.md b/src/lightning/fabric/CHANGELOG.md index 4ce80bb6451e3..18537ca15e2fc 100644 --- a/src/lightning/fabric/CHANGELOG.md +++ b/src/lightning/fabric/CHANGELOG.md @@ -16,6 +16,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). - +### Changed + +- Raise ValueError when seed is `out-of-bounds` or `cannot be cast to int` ([#21029](https://github.com/Lightning-AI/pytorch-lightning/pull/21029)) + --- From 29357f96edf30b5396cbac3fc9207c4572d66658 Mon Sep 17 00:00:00 2001 From: Deependu Jha Date: Tue, 5 Aug 2025 04:32:20 +0530 Subject: [PATCH 8/8] update --- tests/tests_fabric/utilities/test_seed.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/tests_fabric/utilities/test_seed.py b/tests/tests_fabric/utilities/test_seed.py index ecd53fa4521be..2700213747f9a 100644 --- a/tests/tests_fabric/utilities/test_seed.py +++ b/tests/tests_fabric/utilities/test_seed.py @@ -60,6 +60,18 @@ def test_out_of_bounds_seed(seed): seed_everything(seed) +def test_seed_everything_accepts_valid_seed_argument(): + """Ensure that seed_everything returns the provided valid seed.""" + seed_value = 45 + assert seed_everything(seed_value) == seed_value + + +@mock.patch.dict(os.environ, {"PL_GLOBAL_SEED": "17"}, clear=True) +def test_seed_everything_accepts_valid_seed_from_env(): + """Ensure that seed_everything uses the valid seed from the PL_GLOBAL_SEED environment variable.""" + assert seed_everything() == 17 + + def test_reset_seed_no_op(): """Test that the reset_seed function is a no-op when seed_everything() was not used.""" assert "PL_GLOBAL_SEED" not in os.environ