Skip to content

Conversation

Copy link

Copilot AI commented Nov 27, 2025

  • Understand the issue: In parallel Monte Carlo simulation, the "current iteration" display shows inconsistent values because sim_idx is passed directly, but simulations complete out of order
  • Add a completed_count counter to _SimMonitor class to track completed simulations
  • Modify print_update_status() method to use completed_count instead of sim_idx and change label from "Current iteration" to "Iterations completed"
  • Increment completed_count in the method when called (since it's always called within mutex-protected section)
  • Test the changes by running existing tests (both serial and parallel modes pass)
  • Run linter and code review
  • Run CodeQL security check (no issues found)
  • Add entry to CHANGELOG.md
Original prompt

This section details on the original issue you should resolve

<issue_title>BUG: Paralle Monte Carlo simulation giving incorrect iterations</issue_title>
<issue_description>When running the parallel Monte Carlo simulations, there is a printout that shows the current iterations, iteration runtime, and estimated time left. When running in parallel, though, the value goes up and down due to this line

sim_monitor.print_update_status(sim_idx)

This means that the message is printed with the same index as the last completed iteration, so when an earlier iteration takes longer, it updates it with a lower value.

I propose to fix this by having a counter that will keep track and just increment each time. This counter can be incremented when the output is written to a file, and can be thread-safe since it will only be accessed when the mutex is acquired. Also changing the printout of current iteration to iterations completed or something of that nature.

Screenshots

Screen.Recording.2025-04-18.at.22.14.18.mov

</issue_description>

<agent_instructions>Please research what is the best way to solve this problem on Python considering the current implementation of our parallel monte carlo.

</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@Gui-FernandesBR > I propose to fix this by having a counter that will keep track and just increment each time. This counter can be incremented when the output is written to a file, and can be thread-safe since it will only be accessed when the mutex is acquired. Also changing the printout of `current iteration` to iterations completed or something of that nature.

Thanks for reporting this bug, @emtee14 , it seems like you have a great idea of what should be done. Are you able to implement and test it on Python?
Could you kindly submit a Pull Request so we can evaluate your solution please?

Thanks for helping!</comment_new>
<comment_new>@Gui-FernandesBR

Yeah I'm happy to do that

Great!
@emtee14 I have assigned the issue to you then.
Let us know in case you need any help to proceed.</comment_new>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Fix incorrect iteration output in parallel Monte Carlo simulation Fix parallel Monte Carlo simulation showing incorrect iteration count Nov 27, 2025
Copilot finished work on behalf of Gui-FernandesBR November 27, 2025 16:23
@Gui-FernandesBR Gui-FernandesBR marked this pull request as ready for review November 27, 2025 16:27
@Gui-FernandesBR Gui-FernandesBR requested a review from a team as a code owner November 27, 2025 16:27
@Gui-FernandesBR
Copy link
Member

@copilot don't forget to include this PR in the CHANGELOG.md file! Keep the same formatting please.

Copy link
Author

Copilot AI commented Nov 27, 2025

@copilot don't forget to include this PR in the CHANGELOG.md file! Keep the same formatting please.

Added in 8d3e803.

Copilot finished work on behalf of Gui-FernandesBR November 27, 2025 16:30
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

It would be nice to test it before closing the associated issue

@codecov
Copy link

codecov bot commented Nov 27, 2025

Codecov Report

❌ Patch coverage is 11.11111% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.35%. Comparing base (9cf3dd4) to head (7cfc5a5).
⚠️ Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
rocketpy/simulation/monte_carlo.py 11.11% 8 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #885      +/-   ##
===========================================
+ Coverage    80.27%   80.35%   +0.08%     
===========================================
  Files          104      106       +2     
  Lines        12769    13010     +241     
===========================================
+ Hits         10250    10454     +204     
- Misses        2519     2556      +37     

☔ 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.

Copilot AI and others added 2 commits November 27, 2025 13:48
…y increasing completed count

Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
Co-authored-by: Gui-FernandesBR <63590233+Gui-FernandesBR@users.noreply.github.com>
@Gui-FernandesBR Gui-FernandesBR force-pushed the copilot/fix-parallel-monte-carlo-iterations branch from 8d3e803 to 7cfc5a5 Compare November 27, 2025 16:48
@Gui-FernandesBR Gui-FernandesBR merged commit 3cf266b into develop Nov 27, 2025
7 checks passed
@Gui-FernandesBR Gui-FernandesBR deleted the copilot/fix-parallel-monte-carlo-iterations branch November 27, 2025 16:49
@github-project-automation github-project-automation bot moved this from Backlog to Closed in LibDev Roadmap Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

BUG: Paralle Monte Carlo simulation giving incorrect iterations

2 participants