Repository: optuna/optuna
Pull request: Use a fixed seed value when `seed=None` in `GridSampler`
<!-- Thank you for creating a pull request! In general, we merge your pull request after it gets two or more approvals. To proceed to the review process by the maintainers, please make sure that the PR meets the following conditions: (1) it passes all CI checks, and (2) it is neither in the draft nor WIP state. If you wish to discuss the PR in the draft state or need any other help, please mention the Optuna development team in the PR. -->

## Motivation
<!-- Describe your motivation why you will submit this PR. This is useful for reviewers to understand the context of PR. -->

Resolve https://github.com/optuna/optuna/issues/5489. I'm happy to hear the opinion on this change because I'm not sure why this shuffle was introduced. 

## Description of the changes
<!-- Describe the changes in this PR. -->

~Remove the shuffle.~

For distributed optimisation, optuna docs suggest not setting `seed` for samplers or set different values. In this PR, call `shuffle` when `seed=0`.

Problem statement:
GridSampler: Duplicates
### Expected behavior

I use the GridSampler.
In 3.2.0 it runs fine. I upgraded to 3.6.1, and now I see duplicates. If I run the code below 3 times, I might get x=0, then x=1, and then x=0 again. I hadn't such an issue with Optuna 3.2.0

```
import time
import optuna
import random
print(f"optuna: {optuna.__version__}")

def objective(trial: optuna.Trial):
    x = trial.suggest_int("x", -100, 100)
    print("Using parameters:", trial.params)
    time.sleep(20)
    return x**2

if __name__ == '__main__':
    search_space = { "x": [0, 1, 2] }
    sampler = optuna.samplers.GridSampler(search_space)
    storage = optuna.storages.RDBStorage(url="sqlite:////home/me/a.db", engine_kwargs={"connect_args": {"timeout": 100}})
    study = optuna.create_study(storage=storage,
                                sampler=sampler,
                                direction="minimize",
                                pruner=optuna.pruners.NopPruner(),
                                study_name="mystudy",
                                load_if_exists=True)
    study.optimize(objective, n_trials=3, n_jobs=1, callbacks=[optuna.study.MaxTrialsCallback(3, states=None)])
```

### Environment

- Optuna version:3.6.1
- Python version:3.11.4
- OS:Linux-5.14.21-150500.55.39-default-x86_64-with-glibc2.31

### Error messages, stack traces, or logs

```shell
python3 a.py&
[1] 4927
[I 2024-06-11 22:22:22,097] A new study created in RDB with name: mystudy
Using parameters: {'x': 0}

python3 a.py&
[2] 5026
[I 2024-06-11 22:22:26,466] Using an existing study with name 'mystudy' instead of creating a new one.
Using parameters: {'x': 1}

python3 a.py&
[3] 5117
[I 2024-06-11 22:22:31,327] Using an existing study with name 'mystudy' instead of creating a new one.
Using parameters: {'x': 0}
```


### Steps to reproduce

Run the code above.

### Additional context (optional)

_No response_

Hints from issue discussion:
Thank you for sharing the reproducible code! I think this is not a bug. As explained in https://optuna.readthedocs.io/en/stable/tutorial/20_recipes/001_rdb.html#resume-study, the sampler state is not stored in the storage. 


Sorry, I misunderstood the issue description. I think the distributed optimisation is performed here. In this case, as noted in https://optuna.readthedocs.io/en/stable/reference/samplers/generated/optuna.samplers.GridSampler.html 

> A parameter configuration in the grid is not considered finished until its trial is finished. Therefore, during distributed optimization where trials run concurrently, different workers will occasionally suggest the same parameter configuration. The total number of actual trials may therefore exceed the size of the grid.


The issue is that we went from "occasionally suggesting the same parameters" in 3.2.0 (this happened very rarely as far as I can tell) to "suggesting the same parameters is the default" in 3.6.1. 
Hi, thank you for your clarification. I'm not sure but can you specify `seed` of `GridSampler` as a workaround? 

I suspect that https://github.com/optuna/optuna/blob/master/optuna/samplers/_grid.py#L126 is executed with different random seeds without specifying `seed` as a results, each process's grid sampler has different grid id to grid param mapping.
Indeed, by fixing the seed, it works.
Big thanks for your help!
I will close this issue.
Nevertheless, it is surprising that it works fine with version 3.2.0 without setting the seed.

Commit message: Update docstring of `seed`

Patch to review:
diff --git a/optuna/samplers/_grid.py b/optuna/samplers/_grid.py
index accf7601d95..0aa51d3735b 100644
--- a/optuna/samplers/_grid.py
+++ b/optuna/samplers/_grid.py
@@ -102,10 +102,13 @@ def objective(trial):
             A dictionary whose key and value are a parameter name and the corresponding candidates
             of values, respectively.
         seed:
-            A seed to fix the order of trials as the grid is randomly shuffled. Please note that
-            it is not recommended using this option in distributed optimization settings since
-            this option cannot ensure the order of trials and may increase the number of duplicate
-            suggestions during distributed optimization.
+            A seed to fix the order of trials as the grid is randomly shuffled. This shuffle is
+            beneficial when the number of grids is larger than ``n_trials`` in
+            :meth:`~optuna.Study.optimize` not to suggest similar grids. Please note that it is
+            recommended using the same ``seed`` or :obj:`None` in distributed optimization
+            settings because internally each grid is assigned to an integer after shuffling grids.
+            Otherwise, this sampler may increase the number of duplicate suggestions during
+            distributed optimization.
     """
 
     def __init__(
@@ -122,8 +125,9 @@ def __init__(
         self._all_grids = list(itertools.product(*self._search_space.values()))
         self._param_names = sorted(search_space.keys())
         self._n_min_trials = len(self._all_grids)
-        self._rng = LazyRandomState(seed)
-        self._rng.rng.shuffle(self._all_grids)  # type: ignore[arg-type]
+        self._rng = LazyRandomState(seed or 0)
+        if seed is not None:
+            self._rng.rng.shuffle(self._all_grids)  # type: ignore[arg-type]
 
     def reseed_rng(self) -> None:
         self._rng.rng.seed()


Code review comments:
diff_hunk: @@ -122,8 +125,9 @@ def __init__(
         self._all_grids = list(itertools.product(*self._search_space.values()))
         self._param_names = sorted(search_space.keys())
         self._n_min_trials = len(self._all_grids)
-        self._rng = LazyRandomState(seed)
-        self._rng.rng.shuffle(self._all_grids)  # type: ignore[arg-type]
+        self._rng = LazyRandomState(seed or 0)
+        if seed is not None:
+            self._rng.rng.shuffle(self._all_grids)  # type: ignore[arg-type]
path: optuna/samplers/_grid.py
text: @user1:
How about always shuffling? I think it is not proper to use the grid in order only when `seed=None`.

```suggestion
        if seed is None:
            seed = 0
        self._rng = LazyRandomState(seed)
        self._rng.rng.shuffle(self._all_grids)  # type: ignore[arg-type]
```

diff_hunk: @@ -102,10 +102,13 @@ def objective(trial):
             A dictionary whose key and value are a parameter name and the corresponding candidates
             of values, respectively.
         seed:
-            A seed to fix the order of trials as the grid is randomly shuffled. Please note that
-            it is not recommended using this option in distributed optimization settings since
-            this option cannot ensure the order of trials and may increase the number of duplicate
-            suggestions during distributed optimization.
+            A seed to fix the order of trials as the grid is randomly shuffled. This shuffle is
+            beneficial when the number of grids is larger than ``n_trials`` in
+            :meth:`~optuna.Study.optimize` not to suggest similar grids. Please note that it is
+            recommended using the same ``seed`` or :obj:`None` in distributed optimization
+            settings because internally each grid is assigned to an integer after shuffling grids.
+            Otherwise, this sampler may increase the number of duplicate suggestions during
+            distributed optimization.
path: optuna/samplers/_grid.py
text: @user1:
I modified the expressions.
```suggestion
            A seed to fix the order of trials as the grid is randomly shuffled. This shuffle is
            beneficial when the number of grids is larger than ``n_trials`` in
            :meth:`~optuna.Study.optimize` to suppress suggesting similar grids. If ``seed`` is
            :obj:`None`, the shuffle does not happen. Please note that fixing ``seed`` for each
            process is strongly recommended in distributed optimization to avoid duplicated
            suggestions.
```