Skip to content

Commit 893ce8a

Browse files
committed
BUG: refuse a checkpoint whose halves do not name one study
A row carrying run_root: null read as an empty log, so an append started a second root behind it, which is what this check exists to prevent. Rows agreeing on a root also did not make it one a stream can be rebuilt from: SeedSequence(entropy=None) draws fresh entropy every resume. The output rows now carry the root's digest, so an output log from another study with the same indices is refused rather than continued, and both logs are checked against each other before the run decides where to carry on from. A digest rather than the root itself, which would cost 122 bytes a row on a study the input log already records it for. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent ba3bfd3 commit 893ce8a

2 files changed

Lines changed: 218 additions & 14 deletions

File tree

‎rocketpy/simulation/monte_carlo.py‎

Lines changed: 96 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"""
1515

1616
import csv
17+
import hashlib
1718
import json
1819
import os
1920
import traceback
@@ -96,8 +97,75 @@ def _root_written_into_a_row(root_state):
9697
}
9798

9899

99-
def _root_a_log_was_written_with(path):
100-
"""The root every row of a log agrees on, or ``None`` if it holds none.
100+
_ROOT_FIELDS = frozenset(("entropy", "spawn_key", "pool_size", "n_children_spawned"))
101+
102+
103+
def _root_digest(root):
104+
"""A short stable name for a root, for rows that only have to match one.
105+
106+
Carried by the output rows instead of the root itself, which would cost a
107+
hundred bytes a row on a study the input log already records it for.
108+
"""
109+
canonical = json.dumps(root, sort_keys=True, separators=(",", ":"))
110+
return hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:16]
111+
112+
113+
def _a_whole_number(value):
114+
"""A non-negative int, and not a bool standing in for one."""
115+
return isinstance(value, int) and not isinstance(value, bool) and value >= 0
116+
117+
118+
def _whole_numbers(value, may_be_empty=False):
119+
"""A list of non-negative ints, empty only where that is a valid one."""
120+
return (
121+
isinstance(value, list)
122+
and (may_be_empty or len(value) > 0)
123+
and all(_a_whole_number(part) for part in value)
124+
)
125+
126+
127+
def _root_state_a_row_records(root, path):
128+
"""The four values a root is rebuilt from, refused unless all are usable.
129+
130+
Agreeing rows show the study is one study, not that what they agree on can
131+
be resumed: ``SeedSequence(entropy=None)`` draws fresh entropy every time.
132+
"""
133+
entropy = root.get("entropy") if isinstance(root, dict) else None
134+
usable = (
135+
isinstance(root, dict)
136+
and set(root) == _ROOT_FIELDS
137+
and (_a_whole_number(entropy) or _whole_numbers(entropy))
138+
and _whole_numbers(root.get("spawn_key"), may_be_empty=True)
139+
and _a_whole_number(root.get("pool_size"))
140+
and _a_whole_number(root.get("n_children_spawned"))
141+
)
142+
if usable:
143+
state = (
144+
entropy,
145+
tuple(root["spawn_key"]),
146+
root["pool_size"],
147+
root["n_children_spawned"],
148+
)
149+
try:
150+
# Drawing one child is what proves the pool size numpy will take.
151+
_seed_of_simulation(state, 0)
152+
return state
153+
except ValueError:
154+
pass
155+
raise ValueError(
156+
f"cannot continue {path}: its rows record a root that no stream "
157+
f"can be rebuilt from, so what they were drawn with is unknown."
158+
)
159+
160+
161+
def _rows_a_log_holds(path):
162+
"""How many simulations a log records, blank lines aside."""
163+
with open(path, "r", encoding="utf-8") as recorded:
164+
return sum(1 for line in recorded if line.strip())
165+
166+
167+
def _what_the_rows_say_drew_them(path):
168+
"""What every row of a log agrees drew it, or ``None`` if it holds none.
101169
102170
``None`` means the log holds no rows, and nothing else does. Rows that
103171
carry no root are refused instead: they cannot be shown to be one study,
@@ -116,14 +184,14 @@ def _root_a_log_was_written_with(path):
116184
f"cannot continue {path}: a row cannot be read, so what "
117185
f"produced it cannot be established."
118186
) from error
119-
if _SIMULATION_ROOT_KEY not in row:
187+
root = row.get(_SIMULATION_ROOT_KEY) if isinstance(row, dict) else None
188+
if root is None:
120189
raise ValueError(
121190
f"cannot continue {path}: a row does not say which root "
122191
f"drew it, which is how a study written before this "
123192
f"release looks. Start a new one rather than continuing "
124193
f"one whose rows cannot be checked."
125194
)
126-
root = row[_SIMULATION_ROOT_KEY]
127195
if first is _NOTHING_READ_YET:
128196
first = root
129197
elif root != first:
@@ -733,16 +801,27 @@ def __continue_the_root_the_rows_carry(self, random_seed):
733801
also means a fresh object can continue a study, which is the ordinary
734802
way of resuming one.
735803
"""
736-
recorded = _root_a_log_was_written_with(self.input_file)
804+
recorded = _what_the_rows_say_drew_them(self.input_file)
805+
stamped = _what_the_rows_say_drew_them(self.output_file)
806+
named = None if recorded is None else _root_digest(recorded)
807+
if named != stamped:
808+
raise ValueError(
809+
f"cannot append to {self.input_file}: it and {self.output_file} "
810+
f"were not drawn from the same root, so they are two studies "
811+
f"rather than the two halves of one."
812+
)
737813
if recorded is None:
738814
return
739-
if random_seed is None:
740-
self.__root_state = (
741-
recorded["entropy"],
742-
tuple(recorded["spawn_key"]),
743-
recorded["pool_size"],
744-
recorded["n_children_spawned"],
815+
held = _rows_a_log_holds(self.input_file)
816+
if held != _rows_a_log_holds(self.output_file):
817+
raise ValueError(
818+
f"cannot append to {self.input_file}: it and {self.output_file} "
819+
f"hold different numbers of rows, so where to carry on from "
820+
f"cannot be established."
745821
)
822+
state = _root_state_a_row_records(recorded, self.input_file)
823+
if random_seed is None:
824+
self.__root_state = state
746825
return
747826
if _root_written_into_a_row(self.__root_state) != recorded:
748827
raise ValueError(
@@ -1010,8 +1089,6 @@ def __evaluate_flight_outputs(self, flight, sim_idx):
10101089
export_item: getattr(flight, export_item)
10111090
for export_item in self.export_list
10121091
}
1013-
outputs_dict["index"] = sim_idx
1014-
10151092
if self.data_collector is not None:
10161093
additional_exports = {}
10171094
for key, callback in self.data_collector.items():
@@ -1023,6 +1100,12 @@ def __evaluate_flight_outputs(self, flight, sim_idx):
10231100
) from e
10241101
outputs_dict = outputs_dict | additional_exports
10251102

1103+
# After the collectors: these two say which run the row belongs to.
1104+
outputs_dict["index"] = sim_idx
1105+
outputs_dict[_SIMULATION_ROOT_KEY] = _root_digest(
1106+
_root_written_into_a_row(self.__root_state)
1107+
)
1108+
10261109
return (
10271110
json.dumps(outputs_dict, cls=RocketPyEncoder, **self._export_config) + "\n"
10281111
)

‎tests/unit/simulation/test_append_lineage.py‎

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
import pytest
44

5-
from rocketpy.simulation.monte_carlo import MonteCarlo, _SIMULATION_ROOT_KEY
5+
from rocketpy.simulation.monte_carlo import (
6+
MonteCarlo,
7+
_root_digest,
8+
_SIMULATION_ROOT_KEY,
9+
)
610

711

812
def _study(tmp_path, stem, models):
@@ -112,6 +116,76 @@ def test_a_log_whose_rows_carry_no_root_is_refused(models, tmp_path):
112116
analysis.simulate(4, append=True)
113117

114118

119+
def _rewrite(path, replacement):
120+
"""Put ``replacement`` under the root key of every row of one log."""
121+
with open(path, "r", encoding="utf-8") as written:
122+
rows = [json.loads(line) for line in written if line.strip()]
123+
for row in rows:
124+
row[_SIMULATION_ROOT_KEY] = replacement(row[_SIMULATION_ROOT_KEY])
125+
with open(path, "w", encoding="utf-8") as rewritten:
126+
for row in rows:
127+
rewritten.write(json.dumps(row) + "\n")
128+
129+
130+
def _rewrite_roots(analysis, damage):
131+
"""Damage the recorded root, leaving the two logs still naming one study.
132+
133+
The output rows hold the root's digest, so they are restamped rather than
134+
damaged: otherwise the two logs disagree and that is refused first.
135+
"""
136+
with open(analysis.input_file, "r", encoding="utf-8") as written:
137+
first = json.loads(next(line for line in written if line.strip()))
138+
damaged = damage(first[_SIMULATION_ROOT_KEY])
139+
_rewrite(analysis.input_file, lambda _root: damaged)
140+
if isinstance(damaged, dict):
141+
_rewrite(analysis.output_file, lambda _digest: _root_digest(damaged))
142+
143+
144+
def test_a_log_whose_root_is_null_is_refused(models, tmp_path):
145+
"""A null root is no more of a lineage than no root at all."""
146+
# It read as an empty log, so an append started a second root behind the
147+
# damaged rows, which is the case this check exists to prevent.
148+
analysis = _study(tmp_path, "study", models)
149+
analysis.simulate(2, append=False, random_seed=42)
150+
151+
_rewrite_roots(analysis, lambda root: None)
152+
153+
with pytest.raises(ValueError, match="does not say which root"):
154+
analysis.simulate(4, append=True)
155+
156+
157+
@pytest.mark.parametrize(
158+
"damage",
159+
[
160+
lambda root: {**root, "entropy": None},
161+
lambda root: {**root, "pool_size": 0},
162+
lambda root: {**root, "pool_size": 2},
163+
lambda root: {**root, "n_children_spawned": -1},
164+
lambda root: {**root, "spawn_key": [True]},
165+
lambda root: {key: value for key, value in root.items() if key != "entropy"},
166+
lambda root: {**root, "unexpected": 1},
167+
],
168+
ids=[
169+
"null entropy",
170+
"no pool",
171+
"pool numpy refuses",
172+
"negative base",
173+
"bool key",
174+
"short",
175+
"long",
176+
],
177+
)
178+
def test_a_root_that_no_stream_can_be_rebuilt_from_is_refused(models, tmp_path, damage):
179+
"""Rows agreeing on a root does not make it a root that can be resumed."""
180+
analysis = _study(tmp_path, "study", models)
181+
analysis.simulate(2, append=False, random_seed=42)
182+
183+
_rewrite_roots(analysis, damage)
184+
185+
with pytest.raises(ValueError, match="rebuilt from"):
186+
analysis.simulate(4, append=True)
187+
188+
115189
def test_an_empty_log_is_not_a_log_that_cannot_be_checked(models, tmp_path):
116190
"""The control. Nothing recorded yet is a fresh start, not a refusal."""
117191
analysis = _study(tmp_path, "study", models)
@@ -168,3 +242,50 @@ def test_a_row_that_cannot_be_read_is_refused(models, tmp_path):
168242

169243
with pytest.raises(ValueError, match="cannot be read"):
170244
analysis.simulate(4, append=True)
245+
246+
247+
def test_an_output_log_from_another_study_is_refused(models, tmp_path):
248+
"""Matching indices do not make two files the two halves of one run."""
249+
# Nothing in the indices tells them apart: both studies number their rows
250+
# from zero, so the root is what says they belong together.
251+
one = _study(tmp_path, "one", models)
252+
one.simulate(2, append=False, random_seed=42)
253+
other = _study(tmp_path, "other", models)
254+
other.simulate(2, append=False, random_seed=7)
255+
with open(other.output_file, "r", encoding="utf-8") as theirs:
256+
stolen = theirs.read()
257+
with open(one.output_file, "w", encoding="utf-8") as ours:
258+
ours.write(stolen)
259+
260+
with pytest.raises(ValueError, match="same root"):
261+
one.simulate(4, append=True)
262+
263+
264+
def test_a_checkpoint_whose_halves_disagree_is_refused(models, tmp_path):
265+
"""Where to carry on from is not established by one log alone."""
266+
analysis = _study(tmp_path, "study", models)
267+
analysis.simulate(2, append=False, random_seed=42)
268+
with open(analysis.output_file, "r", encoding="utf-8") as written:
269+
rows = [line for line in written if line.strip()]
270+
with open(analysis.output_file, "w", encoding="utf-8") as trimmed:
271+
trimmed.write(rows[0])
272+
273+
with pytest.raises(ValueError, match="different numbers of rows"):
274+
analysis.simulate(4, append=True)
275+
276+
277+
def test_a_collector_cannot_take_over_the_root(models, tmp_path):
278+
"""The root binds the two logs, so nothing outside the run writes it."""
279+
analysis = _study(tmp_path, "study", models)
280+
analysis.data_collector = {_SIMULATION_ROOT_KEY: lambda _flight: "forged"}
281+
282+
analysis.simulate(2, append=False, random_seed=42)
283+
284+
with open(analysis.output_file, "r", encoding="utf-8") as written:
285+
roots = {
286+
json.dumps(json.loads(line)[_SIMULATION_ROOT_KEY], sort_keys=True)
287+
for line in written
288+
if line.strip()
289+
}
290+
assert roots != {'"forged"'}
291+
assert len(roots) == 1

0 commit comments

Comments
 (0)