ENH: Adaptive Monte Carlo via Convergence Criteria

[Malmahrouqi3]

Pull request type

• Code changes (bugfix, features)
• Code maintenance (refactoring, formatting, tests)
• ReadMe, Docs and GitHub updates

Checklist

• Tests for the changes have been added (if needed)
• Docs have been reviewed and added / updated
• Lint (black rocketpy/ tests/) has passed locally
• All tests (pytest tests -m slow --runslow) have passed locally
• CHANGELOG.md has been updated (if relevant)

Current behavior

#892 where users have to specify a number of simulations for Monte Carlo simulations, which may compel them to set the number as high as possible, exhausting tons of compute inefficiently and needlessly.

New behavior

With the new feature, users can select a specific attribute such that if it meets their desired tolerance midway through, then the rest of sims will instantaneously stop since the acquired flight sims are reasonably sufficient to satisfy the Confidence Interval Width.

test_dispersion.simulate_convergence(
        target_attribute="apogee_time",
        target_confidence=0.95,
        tolerance=0.5,  # in seconds
        max_simulations=1000,
        batch_size=50
    )

Also, here is a quick display of outcomes with Attribute: Apogee (±30 m) & Apogee Time (±0.5 s). Batch size: 50 & 200.

Breaking change

• No

Additional information

I attempted to create unit tests, but I was not successful in that since I could not rely merely on the given input lists. Therefore, I opted for an integration test, where it actually passed locally.

[Gui-FernandesBR]

The implementation is really clean. I like what I see here!!

[Copilot]

Pull request overview

This PR introduces adaptive Monte Carlo simulation functionality that allows simulations to terminate early when a confidence interval criterion is met. Instead of requiring users to specify a fixed number of simulations upfront, the new simulate_convergence() method runs simulations in batches and stops when the confidence interval width for a target attribute falls within a specified tolerance.

Key Changes:

• Added simulate_convergence() method to MonteCarlo class that runs simulations in batches until convergence or a maximum limit
• Added integration test for the new convergence simulation method
• Added example usage in the Monte Carlo notebook documentation

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 15 comments.

File	Description
rocketpy/simulation/monte_carlo.py	Implements the new simulate_convergence() method that runs batched simulations until confidence interval meets tolerance
tests/integration/simulation/test_monte_carlo.py	Adds integration test to verify the convergence simulation behavior
docs/notebooks/monte_carlo_analysis/monte_carlo_class_usage.ipynb	Adds example demonstrating usage of the new convergence-based simulation method
CHANGELOG.md	Documents the new adaptive Monte Carlo feature

[Copilot]

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]

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]

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]

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]

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]

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]

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]

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]

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]

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.

[codecov]

Codecov Report

❌ Patch coverage is 8.33333% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.04%. Comparing base (9cf3dd4) to head (f6166b8).
⚠️ Report is 27 commits behind head on develop.

Files with missing lines	Patch %	Lines
rocketpy/simulation/monte_carlo.py	8.33%	11 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #922      +/-   ##
===========================================
+ Coverage    80.27%   81.04%   +0.76%     
===========================================
  Files          104      107       +3     
  Lines        12769    13845    +1076     
===========================================
+ Hits         10250    11220     +970     
- Misses        2519     2625     +106     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Gui-FernandesBR]

@Malmahrouqi3 please address all Copilot`s comments and fix tests/linters on CI so we can review this PR.

Once again I found the implementation clean and nice. I think this is way to go.