Skip to content

Commit 0923d8a

Browse files
BUG: re-raise KeyboardInterrupt after an interrupted Monte Carlo run (#1177)
* 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. Five more holes sat next to that one, found in review and in the interrupt-point audit it prompted: - __run_in_serial bound inputs_json 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. - It also never cleared inputs_json after a successful append, so Ctrl-C landing in the progress print — or in the next keep_simulating() call — wrote the row that had just committed into the error file as though it never finished. - _append_simulation_record rolled back on Exception, and KeyboardInterrupt does not derive from Exception, so Ctrl-C between the two appends left a one-sided record. - Its rollback also only truncated the inputs file, so an interrupt inside either write left a torn partial row for the reload to die on with JSONDecodeError instead of the interrupt. - The parallel cleanup began only after every worker had started, so Ctrl-C during the startup loop left the already-started workers running with nobody signalling or joining them. Bind inputs_json before the loop and clear it after each committed append, roll both files back on BaseException best-effort, cover the startup loop with the same cleanup path over a started-workers list, and re-raise in both handlers. Catch the interrupt in simulate() so __terminate_simulation() still runs before it leaves: it reloads the logs through the file setters, and set_num_of_loaded_sims is what the documented append=True continuation reads. The shutdown join stays unbounded, and the docstring now says so: the interrupt propagates once every worker finishes the simulation it is in and exits on its own. A worker stuck inside one simulation blocks the interrupt as it already blocked the run; killing it here could tear a half-written row into logs it holds the mutex for, and the bounded fleet shutdown belongs to #1054. The ordinary exception path is unchanged. Add ten regression tests covering both modes, the early interrupt, the preserved rows, the reload, an interrupted run continued with append=True, the between-appends rollback, the torn-write rollback, the committed row staying out of the error file, and the startup-loop cleanup. They run the real __terminate_simulation and assert the state it produces; all fail on develop, and each fix was also reverted individually with only its own test failing. * BUG: preserve interrupted Monte Carlo cleanup --------- Co-authored-by: Gui-FernandesBR <guilherme_fernandes@usp.br>
1 parent d864d4d commit 0923d8a

2 files changed

Lines changed: 469 additions & 15 deletions

File tree

‎rocketpy/simulation/monte_carlo.py‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,11 @@ def simulate(
321321
that reports a failure, and logs that do not hold every simulation
322322
asked for are each refused, since a run that lost work must not be
323323
reported as one that completed.
324+
KeyboardInterrupt
325+
If the run is interrupted. The logs written so far are kept and
326+
reloaded first, so the object agrees with its own files and the
327+
run can be continued with ``append=True``, but the interrupt then
328+
reaches the caller rather than being reported as a finished study.
324329
325330
Notes
326331
-----
@@ -349,12 +354,13 @@ def simulate(
349354

350355
self.__setup_files(append)
351356

352-
if parallel:
353-
self.__run_in_parallel(n_workers)
354-
else:
355-
self.__run_in_serial()
356-
357-
self.__terminate_simulation()
357+
try:
358+
if parallel:
359+
self.__run_in_parallel(n_workers)
360+
else:
361+
self.__run_in_serial()
362+
finally:
363+
self.__terminate_simulation()
358364

359365
def __setup_files(self, append):
360366
"""
@@ -410,16 +416,22 @@ def _append_simulation_record(self, inputs_json, outputs_json):
410416
previous_input_size = os.path.getsize(input_path)
411417
except OSError:
412418
previous_input_size = 0
419+
try:
420+
previous_output_size = os.path.getsize(output_path)
421+
except OSError:
422+
previous_output_size = 0
413423

414424
with open(input_path, "a", encoding="utf-8") as f:
415425
f.write(inputs_json)
416426

417427
try:
418428
with open(output_path, "a", encoding="utf-8") as f:
419429
f.write(outputs_json)
420-
except Exception:
430+
except BaseException:
421431
with open(input_path, "rb+") as f:
422432
f.truncate(previous_input_size)
433+
with open(output_path, "rb+") as f:
434+
f.truncate(previous_output_size)
423435
raise
424436

425437
def __run_in_serial(self):
@@ -435,6 +447,7 @@ def __run_in_serial(self):
435447
n_simulations=self.number_of_simulations,
436448
start_time=time(),
437449
)
450+
inputs_json = ""
438451
try:
439452
while sim_monitor.keep_simulating():
440453
sim_monitor.increment()
@@ -445,15 +458,18 @@ def __run_in_serial(self):
445458
outputs_json = self.__evaluate_flight_outputs(flight, sim_monitor.count)
446459

447460
self._append_simulation_record(inputs_json, outputs_json)
461+
inputs_json = ""
448462

449463
sim_monitor.print_update_status()
450464

451465
sim_monitor.print_final_status()
452466

453467
except KeyboardInterrupt:
454468
print("Keyboard interrupt received. Files saved.")
455-
with open(self._error_file, "a", encoding="utf-8") as f:
456-
f.write(inputs_json)
469+
if inputs_json:
470+
with open(self._error_file, "a", encoding="utf-8") as f:
471+
f.write(inputs_json)
472+
raise
457473

458474
except Exception as error:
459475
print(f"Error on iteration {sim_monitor.count}: {error}")
@@ -530,16 +546,15 @@ def __run_in_parallel(self, n_workers=None):
530546

531547
sim_monitor.print_final_status()
532548

533-
# Handle error from the main process
534-
# pylint: disable=broad-except
535-
except (Exception, KeyboardInterrupt) as error:
549+
# Handle error from the main process. Re-raising unconditionally
550+
# is what makes an interrupted run tell the caller it was cut
551+
# short instead of reporting itself as a finished study.
552+
except (Exception, KeyboardInterrupt):
536553
# Bounded here too. An unbounded join undid the bound above.
537554
_stop_the_workers_still_running(
538555
processes, simulation_error_event, _SHUTDOWN_GRACE_SECONDS
539556
)
540-
541-
if not isinstance(error, KeyboardInterrupt):
542-
raise error
557+
raise
543558

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

0 commit comments

Comments
 (0)