Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
Attention: The newest changes should be on top -->

### Added
- TST: Add acceptance tests for 3DOF flight simulation based on Bella Lui rocket [#914] (https://github.com/RocketPy-Team/RocketPy/pull/914_

- ENH: Adaptive Monte Carlo via Convergence Criteria [#922] (https://github.com/RocketPy-Team/RocketPy/pull/922)
- TST: Add acceptance tests for 3DOF flight simulation based on Bella Lui rocket [#914] (https://github.com/RocketPy-Team/RocketPy/pull/914)
Comment on lines +35 to +36
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent markdown link formatting. Lines 35 and 36 use parentheses for the URL part instead of square brackets, which is incorrect markdown syntax. The format should be #922 to match the pattern used in line 37 and other entries.

Suggested change
- ENH: Adaptive Monte Carlo via Convergence Criteria [#922] (https://github.com/RocketPy-Team/RocketPy/pull/922)
- TST: Add acceptance tests for 3DOF flight simulation based on Bella Lui rocket [#914] (https://github.com/RocketPy-Team/RocketPy/pull/914)
- ENH: Adaptive Monte Carlo via Convergence Criteria [#922](https://github.com/RocketPy-Team/RocketPy/pull/922)
- TST: Add acceptance tests for 3DOF flight simulation based on Bella Lui rocket [#914](https://github.com/RocketPy-Team/RocketPy/pull/914)

Copilot uses AI. Check for mistakes.
- ENH: Add background map auto download functionality to Monte Carlo plots [#896](https://github.com/RocketPy-Team/RocketPy/pull/896)
- MNT: net thrust addition to 3 dof in flight class [#907] (https://github.com/RocketPy-Team/RocketPy/pull/907)
- ENH: 3-dof lateral motion improvement [#883](https://github.com/RocketPy-Team/RocketPy/pull/883)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,28 @@
")"
]
},
{
"cell_type": "markdown",
"metadata": {},
"source": [
"Alternatively, we can target an attribute using the method `MonteCarlo.simulate_convergence()` such that when the tolerance is met, the flight simulations would terminate early."
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"test_dispersion.simulate_convergence(\n",
" target_attribute=\"apogee_time\",\n",
" target_confidence=0.95,\n",
" tolerance=0.5, # in seconds\n",
" max_simulations=1000,\n",
" batch_size=50\n",
" )"
Comment on lines +816 to +822
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The notebook example calls simulate_convergence immediately after calling simulate with the same test_dispersion object. This could be confusing because simulate_convergence will append to the existing 1000 simulations from the previous simulate call. Consider clarifying in the documentation that simulate_convergence should be used as an alternative to simulate, or demonstrating it with a fresh MonteCarlo object to avoid confusion.

Copilot uses AI. Check for mistakes.
]
},
{
"attachments": {},
"cell_type": "markdown",
Expand Down
63 changes: 63 additions & 0 deletions rocketpy/simulation/monte_carlo.py
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,69 @@ def estimate_confidence_interval(

return res.confidence_interval

def simulate_convergence(
self,
target_attribute="apogee_time",
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method doesn't validate that target_attribute is a valid attribute name before entering the simulation loop. If an invalid attribute is provided, the error will only be caught after running the first batch of simulations when estimate_confidence_interval is called. Consider validating the target_attribute against a list of valid attributes (similar to checking against self.export_list) before starting the simulation loop to provide faster feedback to users.

Copilot uses AI. Check for mistakes.
target_confidence=0.95,
tolerance=0.5,
max_simulations=1000,
batch_size=50,
parallel=True,
n_workers=4,
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value for n_workers is inconsistent with other methods. The simulate method uses n_workers=None as default (which falls back to os.cpu_count()), but this method defaults to 4. For consistency, consider using n_workers=None and letting the __validate_number_of_workers method handle the default behavior.

Copilot uses AI. Check for mistakes.
):
Comment on lines +528 to +537
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation for critical parameters. The method should validate that target_confidence is between 0 and 1, tolerance is positive, max_simulations is positive, and batch_size is positive and less than or equal to max_simulations. This validation pattern is used in the estimate_confidence_interval method and should be applied here for consistency and robustness.

Copilot uses AI. Check for mistakes.
"""Run simulations cumulatively in batches until the confidence interval meets tolerance.

Parameters
----------
target_attribute : str
The target attribute to track its convergence (e.g., "apogee", "apogee_time", etc.).
target_confidence : float, optional
The confidence level for the interval (between 0 and 1). Default is 0.95.
tolerance : float, optional
The desired width of the confidence interval in seconds, meters, or other units. Default is 0.5.
max_simulations : int, optional
The maximum number of simulations to run to avoid infinite loops. Default is 1000.
batch_size : int, optional
The number of simulations to run in each batch. Default is 50.
parallel : bool, optional
Whether to run simulations in parallel. Default is True.
n_workers : int, optional
The number of worker processes to use if running in parallel. Default is 8.
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring description says the default value is 8 for n_workers, but the actual default in the function signature is 4. This inconsistency should be corrected to match the actual default value.

Suggested change
The number of worker processes to use if running in parallel. Default is 8.
The number of worker processes to use if running in parallel. Default is 4.

Copilot uses AI. Check for mistakes.

Returns
-------
confidence_interval_width : float
The confidence interval width when the simulation stopped for either meeting the tolerance or maximum number of simulations.
Comment on lines +559 to +560
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Returns section of the docstring is incorrect. It states that the method returns a single float value for the confidence interval width, but the actual implementation returns a list of confidence interval widths (one for each batch). The docstring should be updated to reflect that it returns a list of floats representing the confidence interval width history.

Suggested change
confidence_interval_width : float
The confidence interval width when the simulation stopped for either meeting the tolerance or maximum number of simulations.
confidence_interval_width_history : list of float
History of confidence interval widths, one value per batch of simulations.
The last element corresponds to the width when the simulation stopped for
either meeting the tolerance or reaching the maximum number of simulations.

Copilot uses AI. Check for mistakes.
Comment on lines +538 to +560
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring describes the method purpose but could be more precise. The phrase "Run simulations cumulatively in batches" could be clarified to specify that it runs simulations until convergence is achieved or max_simulations is reached. Consider rephrasing to: "Run Monte Carlo simulations in batches until the confidence interval width converges within the specified tolerance or the maximum number of simulations is reached."

Suggested change
"""Run simulations cumulatively in batches until the confidence interval meets tolerance.
Parameters
----------
target_attribute : str
The target attribute to track its convergence (e.g., "apogee", "apogee_time", etc.).
target_confidence : float, optional
The confidence level for the interval (between 0 and 1). Default is 0.95.
tolerance : float, optional
The desired width of the confidence interval in seconds, meters, or other units. Default is 0.5.
max_simulations : int, optional
The maximum number of simulations to run to avoid infinite loops. Default is 1000.
batch_size : int, optional
The number of simulations to run in each batch. Default is 50.
parallel : bool, optional
Whether to run simulations in parallel. Default is True.
n_workers : int, optional
The number of worker processes to use if running in parallel. Default is 8.
Returns
-------
confidence_interval_width : float
The confidence interval width when the simulation stopped for either meeting the tolerance or maximum number of simulations.
"""Run Monte Carlo simulations in batches until the confidence interval
width converges within the specified tolerance or the maximum number of
simulations is reached.
Parameters
----------
target_attribute : str
The target attribute to track its convergence (e.g., "apogee",
"apogee_time", etc.).
target_confidence : float, optional
The confidence level for the interval (between 0 and 1). Default is
0.95.
tolerance : float, optional
The desired width of the confidence interval in seconds, meters, or
other units. Default is 0.5.
max_simulations : int, optional
The maximum number of simulations to run to avoid infinite loops.
Default is 1000.
batch_size : int, optional
The number of simulations to run in each batch. Default is 50.
parallel : bool, optional
Whether to run simulations in parallel. Default is True.
n_workers : int, optional
The number of worker processes to use if running in parallel.
Default is 8.
Returns
-------
confidence_interval_widths : list of float
Sequence of confidence interval widths computed after each batch of
simulations, up to the point where the tolerance criterion is met
or the maximum number of simulations is reached.

Copilot uses AI. Check for mistakes.
"""

self.import_outputs(self.filename.with_suffix(".outputs.txt"))
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling import_outputs at the start of the method may not be necessary and could cause confusion. This loads outputs from an existing file, which may not align with the intent of a fresh convergence simulation. Consider removing this line or documenting why it's necessary to load previous outputs before starting the convergence simulation.

Copilot uses AI. Check for mistakes.
confidence_interval = []

while (self.num_of_loaded_sims < max_simulations):
total_sims = min(self.num_of_loaded_sims + batch_size, max_simulations)

self.simulate(
number_of_simulations=total_sims,
Comment on lines +566 to +570
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The while loop condition could potentially result in running more simulations than max_simulations. If num_of_loaded_sims is less than max_simulations by less than batch_size, the next batch will run total_sims which equals max_simulations (due to the min() operation), but this could mean more work than expected if the confidence interval hasn't converged. Consider adding validation to ensure batch_size doesn't exceed max_simulations, and clarify the behavior when num_of_loaded_sims is not zero at the start.

Suggested change
while (self.num_of_loaded_sims < max_simulations):
total_sims = min(self.num_of_loaded_sims + batch_size, max_simulations)
self.simulate(
number_of_simulations=total_sims,
if max_simulations <= 0:
raise ValueError("max_simulations must be a positive integer.")
if batch_size <= 0:
raise ValueError("batch_size must be a positive integer.")
if batch_size > max_simulations:
warnings.warn(
"batch_size is greater than max_simulations; "
"capping batch_size to max_simulations.",
UserWarning,
)
batch_size = max_simulations
while self.num_of_loaded_sims < max_simulations:
remaining_sims = max_simulations - self.num_of_loaded_sims
sims_to_run = min(batch_size, remaining_sims)
if sims_to_run <= 0:
break
self.simulate(
number_of_simulations=sims_to_run,

Copilot uses AI. Check for mistakes.
append=True,
include_function_data=False,
parallel=parallel,
n_workers=n_workers,
)

self.import_outputs(self.filename.with_suffix(".outputs.txt"))

Comment on lines +577 to +578
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import_outputs call within the loop is inefficient because it reloads and reprocesses the entire output file after each batch. Since simulate() with append=True already writes to the file and the output_file setter triggers set_outputs_log(), set_num_of_loaded_sims(), set_results(), and set_processed_results(), this additional import is redundant. Consider removing this line to improve performance.

Suggested change
self.import_outputs(self.filename.with_suffix(".outputs.txt"))

Copilot uses AI. Check for mistakes.
ci = self.estimate_confidence_interval(
attribute=target_attribute,
confidence_level=target_confidence,
)

Comment on lines +579 to +583
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method does not validate that there are enough simulations to compute a meaningful confidence interval. The bootstrap method in estimate_confidence_interval requires a minimum number of samples to produce reliable results. Consider adding a check to ensure that at least a minimum number of simulations (e.g., batch_size or some reasonable threshold) have been completed before calculating the confidence interval, or document this requirement.

Copilot uses AI. Check for mistakes.
confidence_interval.append(float(ci.high - ci.low))

if float(ci.high - ci.low) <= tolerance:
break

return confidence_interval

def __evaluate_flight_inputs(self, sim_idx):
"""Evaluates the inputs of a single flight simulation.

Expand Down
28 changes: 28 additions & 0 deletions tests/integration/simulation/test_monte_carlo.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,31 @@ def invalid_data_collector(flight):
monte_carlo_calisto.simulate(number_of_simulations=10, append=False)
finally:
_post_test_file_cleanup()


@pytest.mark.slow
def test_monte_carlo_simulate_convergence(monte_carlo_calisto):
"""Tests the simulate_convergence method of the MonteCarlo class.

Parameters
----------
monte_carlo_calisto_pre_loaded : MonteCarlo
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring parameter name is 'monte_carlo_calisto_pre_loaded' but the actual fixture used is 'monte_carlo_calisto'. The docstring should match the actual parameter name used in the function signature.

Suggested change
monte_carlo_calisto_pre_loaded : MonteCarlo
monte_carlo_calisto : MonteCarlo

Copilot uses AI. Check for mistakes.
The MonteCarlo object, this is a pytest fixture.
"""
try:
ci_history = monte_carlo_calisto.simulate_convergence(
target_attribute="apogee",
target_confidence=0.95,
tolerance=5.0,
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tolerance value used in the test appears quite large (5.0 meters for apogee). For a test that verifies convergence behavior, consider using a more realistic tolerance value that would actually test the convergence logic, or add a comment explaining why this large tolerance is appropriate for this test case.

Copilot uses AI. Check for mistakes.
max_simulations=20,
batch_size=5,
parallel=False,
)

assert isinstance(ci_history, list)
print(ci_history)
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The print statement should be removed from the test. Print statements in tests are generally not recommended as they clutter test output and don't provide value in automated testing. If debugging information is needed, use proper logging or remove the statement after debugging is complete.

Suggested change
print(ci_history)

Copilot uses AI. Check for mistakes.
assert all(isinstance(width, float) for width in ci_history)
assert len(ci_history) >= 1
assert monte_carlo_calisto.num_of_loaded_sims <= 20
finally:
_post_test_file_cleanup()
Loading