Skip to content

Commit b5cf7eb

Browse files
BUG: re-raise KeyboardInterrupt after an interrupted Monte Carlo run
simulate() returned normally after Ctrl-C. Both execution modes caught the interrupt and neither re-raised it, so simulate() went on to __terminate_simulation() and returned exactly as it does after a complete study. A caller could not tell a partial run from a finished one without opening the output file and counting rows. __run_in_serial also appended inputs_json to the error file with that name bound inside the loop body, so Ctrl-C during the first keep_simulating() call reached the handler with nothing bound and the run died with UnboundLocalError from inside the cleanup. Bind inputs_json before the loop and re-raise in both handlers. The parallel handler already signalled and joined the workers for both failure kinds; only the re-raise was conditional, so a bare raise covers both and drops the raise-error rebinding. Catch the interrupt in simulate() so __terminate_simulation() still runs before it leaves. That call reloads the logs through the file setters, and set_num_of_loaded_sims is what the documented append=True continuation reads; re-raising past it would leave the object disagreeing with its own files. The ordinary exception path is unchanged. Add six regression tests covering both modes, the early interrupt, the preserved rows, the reload, and an interrupted run continued with append=True. They run the real __terminate_simulation rather than a stub, so they assert the state the reload produces instead of the fact that it was called. The parallel one drives __run_in_parallel over stubs for _import_multiprocess and _create_multiprocess_manager, so it starts no processes and stays deterministic in the default suite.
1 parent 4263fa9 commit b5cf7eb

5 files changed

Lines changed: 259 additions & 8 deletions

File tree

‎monte_carlo_test.errors.txt‎

Whitespace-only changes.

‎monte_carlo_test.inputs.txt‎

Whitespace-only changes.

‎monte_carlo_test.outputs.txt‎

Whitespace-only changes.

‎rocketpy/simulation/monte_carlo.py‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,13 @@ def simulate(
300300
-------
301301
None
302302
303+
Raises
304+
------
305+
KeyboardInterrupt
306+
If the run is interrupted. The completed simulations are saved and
307+
loaded first, then the interrupt is re-raised so that the caller can
308+
tell a partial run from a complete one.
309+
303310
Notes
304311
-----
305312
If you need to stop the simulations after starting them, you can
@@ -327,10 +334,19 @@ def simulate(
327334

328335
self.__setup_files(append)
329336

330-
if parallel:
331-
self.__run_in_parallel(n_workers)
332-
else:
333-
self.__run_in_serial()
337+
try:
338+
if parallel:
339+
self.__run_in_parallel(n_workers)
340+
else:
341+
self.__run_in_serial()
342+
except KeyboardInterrupt:
343+
# The rows that finished are already on disk. Load them before
344+
# leaving, so this object agrees with the files and the documented
345+
# append=True path can continue from them, then let the interrupt
346+
# reach the caller rather than returning as though every requested
347+
# simulation had run.
348+
self.__terminate_simulation()
349+
raise
334350

335351
self.__terminate_simulation()
336352

@@ -413,6 +429,9 @@ def __run_in_serial(self):
413429
n_simulations=self.number_of_simulations,
414430
start_time=time(),
415431
)
432+
# Bound before the loop: Ctrl-C during the first keep_simulating() or
433+
# increment() call reaches the handler below, which reads this name.
434+
inputs_json = ""
416435
try:
417436
while sim_monitor.keep_simulating():
418437
sim_monitor.increment()
@@ -432,6 +451,8 @@ def __run_in_serial(self):
432451
print("Keyboard interrupt received. Files saved.")
433452
with open(self._error_file, "a", encoding="utf-8") as f:
434453
f.write(inputs_json)
454+
# Bare, so the handler's own line does not join the traceback.
455+
raise
435456

436457
except Exception as error:
437458
print(f"Error on iteration {sim_monitor.count}: {error}")
@@ -499,15 +520,17 @@ def __run_in_parallel(self, n_workers=None):
499520
sim_monitor.print_final_status()
500521

501522
# Handle error from the main process
502-
# pylint: disable=broad-except
503-
except (Exception, KeyboardInterrupt) as error:
523+
except (Exception, KeyboardInterrupt):
504524
simulation_error_event.set()
505525

506526
for sim_producer in processes:
507527
sim_producer.join()
508528

509-
if not isinstance(error, KeyboardInterrupt):
510-
raise error
529+
# Ctrl-C is re-raised alongside the other failures now that the
530+
# workers have been signalled and joined. Swallowing it left
531+
# simulate() returning as though the run had completed.
532+
# Bare, so the handler's own line does not join the traceback.
533+
raise
511534

512535
def __validate_number_of_workers(self, n_workers):
513536
if n_workers is None or n_workers > os.cpu_count():

‎tests/unit/simulation/test_monte_carlo.py‎

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import pytest
1313

1414
from rocketpy.simulation import MonteCarlo
15+
from rocketpy.simulation import monte_carlo as mc_module
1516
from rocketpy.simulation.monte_carlo import (
1617
_refuse_logs_this_run_cannot_write,
1718
)
@@ -789,3 +790,230 @@ def test_two_names_for_a_file_that_does_not_exist_yet_are_still_one_file(tmp_pat
789790
assert not pathlib.Path(missing).exists()
790791
with pytest.raises(ValueError, match="same file"):
791792
_refuse_logs_this_run_cannot_write(missing, same_by_another_name, errors)
793+
794+
795+
class _InterruptingMonteCarlo(MonteCarlo):
796+
"""A MonteCarlo that raises ``KeyboardInterrupt`` where Ctrl-C would land.
797+
798+
Only the attributes ``simulate`` and ``__run_in_serial`` touch are set, so no
799+
stochastic object graph or real flight is needed. The name-mangled overrides
800+
stand in for the members ``MonteCarlo`` calls on itself.
801+
"""
802+
803+
# pylint: disable=super-init-not-called,invalid-name,unused-argument
804+
805+
def __init__(self, filename, interrupt_after):
806+
self.filename = filename
807+
self._input_file = filename + ".inputs.txt"
808+
self._output_file = filename + ".outputs.txt"
809+
self._error_file = filename + ".errors.txt"
810+
self.num_of_loaded_sims = 0
811+
self.number_of_simulations = 0
812+
self._export_config = {}
813+
self._initial_sim_idx = 0
814+
self.interrupt_after = interrupt_after
815+
self.completed = 0
816+
817+
def _MonteCarlo__run_single_simulation(self):
818+
if self.completed >= self.interrupt_after:
819+
raise KeyboardInterrupt("ctrl-c")
820+
self.completed += 1
821+
return object()
822+
823+
def _MonteCarlo__evaluate_flight_inputs(self, index):
824+
return json.dumps({"index": index}) + "\n"
825+
826+
def _MonteCarlo__evaluate_flight_outputs(self, flight, index):
827+
return json.dumps({"index": index, "apogee": 1000.0 + index}) + "\n"
828+
829+
830+
def test_interrupted_serial_run_reaches_the_caller(tmp_path):
831+
"""``simulate`` used to return normally after Ctrl-C.
832+
833+
A caller could not tell a partial run from a complete one without opening
834+
the output file and counting rows.
835+
"""
836+
mc = _InterruptingMonteCarlo(str(tmp_path / "run"), interrupt_after=2)
837+
838+
with pytest.raises(KeyboardInterrupt):
839+
mc.simulate(number_of_simulations=10, parallel=False)
840+
841+
842+
def test_interrupted_serial_run_keeps_the_rows_that_finished(tmp_path):
843+
"""The two simulations that completed stay readable and paired."""
844+
mc = _InterruptingMonteCarlo(str(tmp_path / "run"), interrupt_after=2)
845+
846+
with pytest.raises(KeyboardInterrupt):
847+
mc.simulate(number_of_simulations=10, parallel=False)
848+
849+
inputs = (tmp_path / "run.inputs.txt").read_text(encoding="utf-8").splitlines()
850+
outputs = (tmp_path / "run.outputs.txt").read_text(encoding="utf-8").splitlines()
851+
852+
assert len(inputs) == 2
853+
assert len(outputs) == 2
854+
assert [json.loads(row)["index"] for row in inputs] == [
855+
json.loads(row)["index"] for row in outputs
856+
]
857+
858+
859+
def test_interrupted_serial_run_still_reloads_the_logs(tmp_path):
860+
"""``__terminate_simulation`` runs before the interrupt leaves ``simulate``.
861+
862+
It is what reloads the logs through the file setters. Asserting on the
863+
state those setters produce, rather than on the call, is what shows the
864+
reload actually happened.
865+
"""
866+
mc = _InterruptingMonteCarlo(str(tmp_path / "run"), interrupt_after=2)
867+
868+
with pytest.raises(KeyboardInterrupt):
869+
mc.simulate(number_of_simulations=10, parallel=False)
870+
871+
assert mc.num_of_loaded_sims == 2
872+
assert len(mc.inputs_log) == 2
873+
assert len(mc.outputs_log) == 2
874+
assert mc.results["apogee"] == [1001.0, 1002.0]
875+
876+
877+
def test_an_interrupted_run_can_be_continued_with_append(tmp_path):
878+
"""The behavior the ``simulate`` docstring promises after an interrupt.
879+
880+
``set_num_of_loaded_sims`` is what ``append=True`` reads to decide where to
881+
resume, and it is only set by the reload above. This runs the whole path:
882+
interrupt, then continue, and check the indices on disk have no gap and no
883+
repeat.
884+
"""
885+
stem = str(tmp_path / "run")
886+
mc = _InterruptingMonteCarlo(stem, interrupt_after=2)
887+
888+
with pytest.raises(KeyboardInterrupt):
889+
mc.simulate(number_of_simulations=10, parallel=False)
890+
891+
mc.interrupt_after = 10
892+
mc.simulate(number_of_simulations=10, append=True, parallel=False)
893+
894+
rows = pathlib.Path(stem + ".outputs.txt").read_text(encoding="utf-8").splitlines()
895+
assert [json.loads(row)["index"] for row in rows] == list(range(1, 11))
896+
897+
898+
def test_ctrl_c_before_the_first_simulation_is_still_the_interrupt(
899+
tmp_path, monkeypatch
900+
):
901+
"""The handler appends ``inputs_json``, which used to be unbound this early.
902+
903+
Ctrl-C during the first ``keep_simulating()`` call reached the handler
904+
before the loop body had bound the name, so the run died with
905+
``UnboundLocalError`` from inside the cleanup instead of with the interrupt.
906+
"""
907+
908+
def interrupt(self):
909+
raise KeyboardInterrupt("ctrl-c before the first simulation")
910+
911+
monkeypatch.setattr(mc_module._SimMonitor, "keep_simulating", interrupt)
912+
mc = _InterruptingMonteCarlo(str(tmp_path / "run"), interrupt_after=0)
913+
914+
with pytest.raises(KeyboardInterrupt):
915+
mc.simulate(number_of_simulations=5, parallel=False)
916+
917+
918+
class _FakeWorker:
919+
"""Stands in for a ``multiprocess.Process`` without starting anything."""
920+
921+
def __init__(self, interrupt_on_first_join=False):
922+
self.starts = 0
923+
self.joins = 0
924+
self._interrupt_on_first_join = interrupt_on_first_join
925+
926+
def start(self):
927+
self.starts += 1
928+
929+
def join(self):
930+
self.joins += 1
931+
if self._interrupt_on_first_join and self.joins == 1:
932+
raise KeyboardInterrupt("ctrl-c while waiting for the workers")
933+
934+
935+
class _FakeManager:
936+
"""The subset of the multiprocess manager that ``__run_in_parallel`` uses."""
937+
938+
# pylint: disable=invalid-name
939+
940+
def __init__(self):
941+
self.event = _FakeEvent()
942+
self.monitor = _FakeSimMonitor()
943+
944+
def __enter__(self):
945+
return self
946+
947+
def __exit__(self, *_exc):
948+
return False
949+
950+
def Lock(self):
951+
return object()
952+
953+
def Event(self):
954+
return self.event
955+
956+
def _SimMonitor(self, **_kwargs):
957+
return self.monitor
958+
959+
960+
class _FakeEvent:
961+
def __init__(self):
962+
self._set = False
963+
964+
def set(self):
965+
self._set = True
966+
967+
def is_set(self):
968+
return self._set
969+
970+
971+
class _FakeSimMonitor:
972+
def __init__(self, **_kwargs):
973+
self.final_status_calls = 0
974+
975+
def print_final_status(self):
976+
self.final_status_calls += 1
977+
978+
979+
def test_interrupted_parallel_run_signals_joins_and_reaches_the_caller(
980+
tmp_path, monkeypatch
981+
):
982+
"""Ctrl-C while waiting on the workers must not end as a successful run.
983+
984+
The handler already signalled and joined the workers, but it then swallowed
985+
the interrupt, so ``simulate`` went on to report the study as finished.
986+
"""
987+
manager = _FakeManager()
988+
workers = []
989+
990+
class _FakeMultiprocess:
991+
# pylint: disable=invalid-name
992+
@staticmethod
993+
def Process(target=None, args=()): # pylint: disable=unused-argument
994+
worker = _FakeWorker(interrupt_on_first_join=not workers)
995+
workers.append(worker)
996+
return worker
997+
998+
monkeypatch.setattr(
999+
mc_module, "_import_multiprocess", lambda: (_FakeMultiprocess, None)
1000+
)
1001+
monkeypatch.setattr(
1002+
mc_module, "_create_multiprocess_manager", lambda *_args: manager
1003+
)
1004+
mc = _InterruptingMonteCarlo(str(tmp_path / "run"), interrupt_after=0)
1005+
1006+
with pytest.raises(KeyboardInterrupt):
1007+
mc.simulate(number_of_simulations=4, parallel=True, n_workers=2)
1008+
1009+
assert len(workers) == 2
1010+
assert all(worker.starts == 1 for worker in workers)
1011+
assert manager.event.is_set(), "the workers were never told to stop"
1012+
assert workers[0].joins == 2, (
1013+
"the interrupted join was not retried after signalling"
1014+
)
1015+
assert workers[1].joins == 1, "the second worker was never joined"
1016+
assert manager.monitor.final_status_calls == 0, "a partial run reported completion"
1017+
# __init__ above never sets inputs_log; only the reload in
1018+
# __terminate_simulation does, so its presence is what shows it ran.
1019+
assert hasattr(mc, "inputs_log"), "the reload did not run on the interrupted path"

0 commit comments

Comments
 (0)