diff --git a/HISTORY.rst b/HISTORY.rst index ccf4ced..494f186 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -1,6 +1,12 @@ ======= History ======= +2026.10.17.1 -- Warn about a fixed random seed in a loop + * A step in a loop's body that uses a fixed random seed, such as LAMMPS's + Velocities, gives every iteration the same random numbers: iterations that start + from the same structure are then the same run, not independent replicas. The + check before the job runs warns about it, with a fix setting the seed to random. + 2026.10.17 -- Parallel loops: MD runs kept for analysis, and a check before running * The record of the MD runs that analysis steps read (Viscosity, Diffusivity, Liquid Structure, Dielectric Constant, Ionic Conductivity) now comes back from diff --git a/docs/user_guide/index.rst b/docs/user_guide/index.rst index b96b5e5..31a2a9a 100644 --- a/docs/user_guide/index.rst +++ b/docs/user_guide/index.rst @@ -85,6 +85,9 @@ Choosing parallel declares that the iterations are independent: one configuration). The check before the job runs warns about each such step in the body of a parallel loop over one structure, and the editor offers to change it with one click (``seamm-flowchart validate --fix`` does the same). + Replicas need different random numbers too: the check also warns about a step + in the body with a fixed random seed, such as LAMMPS's Velocities, which by + default draws a new one each time. - ``break``, ``continue`` and skipping an iteration work as in a serial loop: after a break in iteration *k* the iterations after *k* are not merged, so the result is the serial one. diff --git a/loop_step/loop.py b/loop_step/loop.py index 5151f8c..ffee1f0 100644 --- a/loop_step/loop.py +++ b/loop_step/loop.py @@ -1303,7 +1303,7 @@ def check_flowchart(self, preceding): [str] The warnings, as seamm.Problem with their fixes; empty if none. """ - return self._overwriting_steps() + return self._overwriting_steps() + self._fixed_seeds() def _overwriting_steps(self): """The warnings for the body's steps that overwrite the shared structure.""" @@ -1347,6 +1347,42 @@ def _overwriting_steps(self): ) return problems + def _fixed_seeds(self): + """Warnings for steps in the body that seed their random numbers with a + fixed value: every iteration gets the same random numbers, so runs that + start from the same structure are the same run.""" + try: + from seamm.problems import WARNING, Edit, Fix, Problem + except ImportError: # seamm before 2026.10.17 + return [] + if _static(self.parameters, "type") in (None, "For systems in the database"): + return [] + problems = [] + for node in self._body_nodes(): + seed = _static(node.parameters, "seed") + if seed is None or str(seed).strip().lower() == "random": + continue + if node.parameters is not None and hasattr(node.parameters, "applies"): + values = {k: p.value for k, p in node.parameters.items()} + if not node.parameters.applies("seed", values): + continue + fix = Fix( + f"Use a random seed in '{node.title}'", + [Edit(node, "seed", "random")], + ) + problems.append( + Problem( + f"'{node.title}' in the loop '{self.title}' uses the fixed random " + f"seed {seed}, so every iteration gets the same random numbers: " + "iterations that start from the same structure are the same run, " + "not independent replicas. Use a random seed, or one that depends " + "on the loop variable.", + severity=WARNING, + fixes=[fix], + ) + ) + return problems + def _body_nodes(self): """The steps in the loop's body, including those inside sub-flowcharts.""" from seamm import format3 diff --git a/tests/test_check_flowchart.py b/tests/test_check_flowchart.py index 5872f07..1e7d0ee 100644 --- a/tests/test_check_flowchart.py +++ b/tests/test_check_flowchart.py @@ -102,3 +102,57 @@ def test_a_step_inside_a_body_step(): def test_no_warning(settings): flowchart, loop, step = make(**settings) assert validate(flowchart, warnings=True) == [] + + +class Velocities(seamm.Node): + """A step with a random seed, as LAMMPS's Velocities sub-step has.""" + + def __init__(self, flowchart, seed="random"): + super().__init__(flowchart=flowchart, title="Velocities", extension="Step") + self.parameters = seamm.Parameters( + defaults={ + "seed": { + "default": "random", + "kind": "integer", + "default_units": None, + "enumeration": ("random",), + "format_string": "", + "description": "Random seed:", + "help_text": "The seed.", + } + } + ) + self.parameters["seed"].value = seed + + +def seeded(seed, parallel="no", kind="For"): + flowchart = seamm.Flowchart() + loop = loop_step.Loop(flowchart=flowchart) + flowchart.add_node(loop) + _connect(flowchart, flowchart.get_node("1"), loop, "next") + step = Velocities(flowchart, seed) + flowchart.add_node(step) + _connect(flowchart, loop, step, "loop") + _connect(flowchart, step, loop, "next") + loop.parameters["parallel"].value = parallel + loop.parameters["type"].value = kind + return flowchart, step + + +@pytest.mark.parametrize("parallel", ["no", "yes"]) +def test_a_fixed_seed_in_a_loop(parallel): + flowchart, step = seeded("12345", parallel) + (problem,) = validate(flowchart, warnings=True) + assert "fixed random seed 12345" in problem + problem.fixes[0].apply() + assert step.parameters["seed"].value == "random" + assert validate(flowchart, warnings=True) == [] + + +@pytest.mark.parametrize( + "seed, kind", + [("random", "For"), ("$seed", "For"), ("12345", "For systems in the database")], +) +def test_no_seed_warning(seed, kind): + flowchart, step = seeded(seed, kind=kind) + assert validate(flowchart, warnings=True) == []