Skip to content

BUG: MonteCarlo returns normally after KeyboardInterrupt #1151

Description

@ting-hong-shieh

Describe the bug

MonteCarlo.simulate() returns normally after the user interrupts a run.

In serial mode, __run_in_serial() catches KeyboardInterrupt, prints that files were saved, appends the current input record to the error file, and then returns. In parallel mode, __run_in_parallel() catches KeyboardInterrupt, signals and joins the workers, but only re-raises non-keyboard exceptions.

The public simulate() method then calls __terminate_simulation() and returns as if the requested number of simulations completed. Callers cannot distinguish a partial run from a successful one without inspecting the output files.

To reproduce

Start a serial or parallel Monte Carlo run and interrupt it with Ctrl-C. At cb6106a717207dd8fc2dfe1446d80ff75022f21b, control returns normally after the cleanup message instead of propagating KeyboardInterrupt.

Expected behavior

RocketPy should preserve completed rows and stop or join active workers, then re-raise the original KeyboardInterrupt. This lets command-line callers return the conventional interrupted status and lets Python callers handle a partial run explicitly.

The saved files should remain usable with the documented append=True continuation path.

Proposed fix

Keep the current cleanup and file-preservation behavior in both execution modes, but re-raise after cleanup. Add serial and parallel regression tests that verify:

  • completed rows remain readable;
  • workers are signalled and joined;
  • __terminate_simulation() does not convert the interrupted run into success;
  • the caller receives KeyboardInterrupt.

The relevant control flow is also being changed by #1054, so implementation should follow that pull request rather than introducing a conflicting parallel-run rewrite.

Environment

  • Base SHA: cb6106a717207dd8fc2dfe1446d80ff75022f21b
  • RocketPy 1.13.0
  • Python 3.12.6
  • NumPy 2.5.2
  • SciPy 1.18.0
  • pytest 9.1.1
  • macOS 26.5.2, arm64

Activity

  1. ting-hong-shieh commented on Aug 14, 2026

    @ting-hong-shieh
    Author

    I reproduced the interrupt behavior while measuring the slow-test coverage at cb6106a717207dd8fc2dfe1446d80ff75022f21b.

    Command:

    $ pytest tests --runslow -m slow --cov=rocketpy --cov-append --cov-report=

    During test_monte_carlo_simulate[True], sending Ctrl-C did not terminate pytest. The Monte Carlo case returned control to the test, was marked as failed, and pytest continued into the next slow case. A second Ctrl-C was handled the same way; I ultimately had to send SIGQUIT to terminate the run.

    This is consistent with both simulation paths catching KeyboardInterrupt and returning normally after cleanup. It also makes an apparently stalled Monte Carlo run difficult to stop from a test runner or terminal.

    Environment:

    • macOS 26.5.2, arm64
    • Python 3.12.6
    • RocketPy 1.13.0
    • NumPy 2.5.2
    • SciPy 1.18.0
    • pytest 9.1.1

    The intended regression test should assert that cleanup and file persistence finish, then that the original KeyboardInterrupt escapes simulate() in both serial and parallel modes. I will keep the implementation behind #1054 so it does not duplicate or conflict with that process-handling work.

  2. ting-hong-shieh commented on Aug 16, 2026

    @ting-hong-shieh
    Author

    Correcting the last paragraph of the report. I wrote that implementation should follow #1054 because that pull request changes the same control flow. It does change it, but it does not close this issue, so the two are independent after all.

    What #1054 does with the interrupt

    Both handlers are rewritten, and both record the interrupt instead of propagating it:

    except KeyboardInterrupt:
        self._interrupted = True
        print("Keyboard interrupt received. Files saved.")
        self.__keep_the_inputs_that_did_not_finish(inputs_json)
    except KeyboardInterrupt:
        _bring_the_fleet_down(started_processes, simulation_error_event)
        self._interrupted = True

    The flag is read in one place, __check_each_index_was_recorded_once, where it suppresses the missing-simulations check:

    missing = (
        []
        if self._interrupted
        else sorted(set(range(self.number_of_simulations)) - set(inputs))
    )

    That is deliberate and correct for what #1054 is about: an interrupted run has gaps in its indices and must not be reported as a corrupt one. But simulate() still runs to its end and returns, so the caller still cannot tell a partial run from a complete one. This issue survives #1054.

    Still reproducing on develop

    At 4263fa95d7fe6f63d9593f01e4ff7a088369e195, serial mode, with __run_single_simulation raising KeyboardInterrupt on the third call where Ctrl-C would:

    Starting Monte Carlo analysis
    Iterations completed: 000001 ...
    Iterations completed: 000002 ...
    Keyboard interrupt received. Files saved.
    __terminate_simulation ran
    
    asked for              : 10 simulations
    completed before Ctrl-C: 2 inputs, 2 outputs
    KeyboardInterrupt seen by caller: False
    

    The two completed rows are intact in both files, which is the behavior worth keeping. What is missing is the last step: simulate() returns as though all ten had run.

    What this changes about the fix

    The proposed fix in the report stands, but the ordering note in it does not. Re-raising after cleanup is a few lines in each handler and does not require the parallel-run rewrite, so it can go in before or after #1054 rather than waiting on it. Whichever lands second is a small rebase.

    A second defect in the same handler

    The serial handler writes inputs_json to the error file, but that name is bound inside the loop body. Ctrl-C during the first keep_simulating() call therefore reaches the handler with nothing bound. At the same commit:

    Starting Monte Carlo analysis
    Keyboard interrupt received. Files saved.
    RAISED UnboundLocalError: cannot access local variable 'inputs_json' where it is not associated with a value
    

    So interrupting early does raise, but it raises the wrong exception from inside the cleanup, after printing that the files were saved.

    #1054 already fixes this by binding inputs_json as the first statement in the loop, with a comment naming exactly this case. Whoever picks this issue up should take that ordering along with the re-raise rather than solving it twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions