BUG: notice a worker that never finished

The workers say they failed by setting an event, and the parent joins
them and reads it. A worker that is killed runs no handler, so the
event stays clear, the join returns because the process is gone, and
the run reports the simulations it never wrote as done.

Measured with a SIGKILL in the second simulation of six, two workers:
simulate() returned normally with two rows on disk.

Its exit code is what is left of a worker that ends this way, so the
parent reads that too. Anything other than zero is refused, None
included, since that is a worker that has not finished at all.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

Author: thc1006

rocketpy/simulation/monte_carlo.py

@@ -488,6 +488,11 @@ def __run_in_parallel(self, n_workers=None):
                 for sim_producer in processes:
                     sim_producer.join()
 
+                # Before the event: a worker that was killed, or that died
+                # before its own handler could set it, leaves it clear, and the
+                # run would report the simulations it never wrote as done.
+                _refuse_a_worker_that_did_not_finish(processes)
+
                 # Handle error from the child processes
                 if simulation_error_event.is_set():
                     raise RuntimeError(
@@ -1758,6 +1763,27 @@ def export_errors_to_json(self, filename):
         self._write_log_to_json(self.errors_log, filename)
 
 
+def _refuse_a_worker_that_did_not_finish(processes):
+    """Raise if any worker left without exiting cleanly.
+
+    The workers report their own failures through an event, which one that was
+    killed never reaches, so what is left of it is its exit code. A negative
+    one is the signal that ended it, and ``None`` is one still running.
+    """
+    unfinished = [
+        f"worker {position} with exit code {process.exitcode}"
+        for position, process in enumerate(processes)
+        if process.exitcode != 0
+    ]
+    if not unfinished:
+        return
+    raise RuntimeError(
+        f"The run is incomplete: {', '.join(unfinished)}. A worker that ends "
+        "this way records nothing and cannot say why, so the simulations it "
+        "held are missing from the results."
+    )
+
+
 def _import_multiprocess():
     """Import the necessary modules and submodules for the
     multiprocess library.

tests/unit/simulation/test_monte_carlo_worker_exit.py

@@ -0,0 +1,67 @@
+import os
+import signal
+from types import SimpleNamespace
+
+import pytest
+
+from rocketpy.simulation import monte_carlo as mc_module
+from rocketpy.simulation.monte_carlo import _refuse_a_worker_that_did_not_finish
+
+
+def _worker(exitcode):
+    return SimpleNamespace(exitcode=exitcode)
+
+
+def test_workers_that_all_exited_cleanly_are_accepted():
+    _refuse_a_worker_that_did_not_finish([_worker(0), _worker(0)])
+
+
+def test_a_worker_killed_by_a_signal_is_refused():
+    with pytest.raises(RuntimeError, match=r"worker 1 with exit code -9"):
+        _refuse_a_worker_that_did_not_finish([_worker(0), _worker(-9)])
+
+
+def test_a_worker_that_exited_nonzero_is_refused():
+    with pytest.raises(RuntimeError, match=r"worker 0 with exit code 1"):
+        _refuse_a_worker_that_did_not_finish([_worker(1), _worker(0)])
+
+
+def test_every_unfinished_worker_is_named():
+    with pytest.raises(RuntimeError) as raised:
+        _refuse_a_worker_that_did_not_finish([_worker(-9), _worker(0), _worker(3)])
+
+    assert "worker 0" in str(raised.value)
+    assert "worker 2" in str(raised.value)
+    assert "worker 1" not in str(raised.value)
+
+
+@pytest.mark.parametrize("exitcode", [None, -15, 2])
+def test_anything_but_a_clean_exit_is_refused(exitcode):
+    with pytest.raises(RuntimeError):
+        _refuse_a_worker_that_did_not_finish([_worker(exitcode), _worker(0)])
+
+
+_REAL_SIMULATION = mc_module.MonteCarlo._MonteCarlo__run_single_simulation
+
+
+def test_a_killed_worker_does_not_pass_as_a_finished_run(monte_carlo_calisto, tmp_path):
+    # The event the workers report through is set by their own handler, and a
+    # signal runs no handler, so the run used to return as though it had done
+    # every simulation it was asked for.
+    calls = {"count": 0}
+
+    def die_on_the_second(self):
+        calls["count"] += 1
+        if calls["count"] == 2:
+            os.kill(os.getpid(), signal.SIGKILL)
+        return _REAL_SIMULATION(self)
+
+    mc_module.MonteCarlo._MonteCarlo__run_single_simulation = die_on_the_second
+    monte_carlo_calisto.filename = os.path.join(str(tmp_path), "study")
+    try:
+        with pytest.raises(RuntimeError, match="incomplete"):
+            monte_carlo_calisto.simulate(
+                number_of_simulations=6, append=False, parallel=True, n_workers=2
+            )
+    finally:
+        mc_module.MonteCarlo._MonteCarlo__run_single_simulation = _REAL_SIMULATION