fixed potential memory leak in parallelization#336
Merged
aaronleesander merged 7 commits intomainfrom Feb 17, 2026
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes refactor parallel task execution in the simulator using a worker-context model, replacing direct parameter passing with shared context objects. This includes thread-limiting logic, worker initializers, and progress tracking across digital and analog simulation paths. A memory leak fix is documented in the changelog. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mqt/yaqs/simulator.py (2)
349-383: 🧹 Nitpick | 🔵 TrivialDuplicate section number in comments — both sections labeled "8)".
Line 350 and Line 383 both use section number
8). Likely a copy-paste oversight.✏️ Fix section numbering
# --------------------------------------------------------------------------- -# 8) MULTIPROCESS "spawn" CONTEXT +# 9) MULTIPROCESS "spawn" CONTEXT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mqt/yaqs/simulator.py` around lines 349 - 383, The comment headers have a duplicated section number "8)" around the _call_backend function; update the comment numbering to be sequential and unique (e.g., change this section to "8)" and the following "8) MULTIPROCESS \"spawn\" CONTEXT" to "9) MULTIPROCESS \"spawn\" CONTEXT" or vice versa) so section labels are not duplicated; locate the section header string containing "_call_backend" and the next header text "MULTIPROCESS \"spawn\" CONTEXT" and adjust the numeral to maintain proper ordering.
41-51: 🧹 Nitpick | 🔵 TrivialDuplicate import of
Callable.
Callableis imported at runtime on Line 41 (from collections.abc import Callable) and again inside theif TYPE_CHECKING:block on Line 51. The TYPE_CHECKING import is redundant since the runtime import already makesCallableavailable everywhere.♻️ Remove the redundant import
if TYPE_CHECKING: - from collections.abc import Callable🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mqt/yaqs/simulator.py` around lines 41 - 51, Remove the redundant type-only import of Callable inside the TYPE_CHECKING block: since Callable is already imported at runtime (from collections.abc import Callable), delete the duplicate "from collections.abc import Callable" line inside the if TYPE_CHECKING: block so Callable is only imported once and type checks still work; verify no other TYPE_CHECKING-only imports rely on that duplicate before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mqt/yaqs/simulator.py`:
- Around line 838-844: Initialize ctx before the conditional to avoid the
"potentially unbound" warning: declare ctx = None (with an appropriate Optional
type annotation) above the if sim_params.solver == "MCWF": block, keep the
existing assignment inside that block, and leave the later use in args as-is
(it's still guarded by the same MCWF branch so no runtime change); update
imports/annotations if needed to reference typing.Optional and the actual ctx
type.
- Around line 834-835: The serial fallback currently sets n_threads =
available_cpus() which ignores the sim_params.num_threads setting; change the
serial path to respect the num_threads attribute on the simulation params (e.g.,
use sim_params.num_threads or default to 1) instead of available_cpus(). Update
the block that assigns n_threads (and any downstream use of n_threads) to read
sim_params.num_threads (falling back to 1 if unset) so StrongSimParams and
AnalogSimParams controls are honored. Ensure available_cpus() continues to be
used only for the parallel path, and keep variable name n_threads unchanged.
- Around line 586-598: The serial execution paths currently set n_threads =
available_cpus(), which overrides the user's StrongSimParams.num_threads
preference; change the serial-path thread selection to use
sim_params.num_threads (falling back to available_cpus() only if num_threads is
missing or None) so BLAS/LAPACK threading honors the user's setting—update the
occurrence in this loop (where n_threads is used with _call_backend(backend,
arg, n_threads=n_threads)) and mirror the same change in the other serial
branches for weak and analog simulations so all serial code uses
sim_params.num_threads instead of unconditionally calling available_cpus().
---
Outside diff comments:
In `@src/mqt/yaqs/simulator.py`:
- Around line 349-383: The comment headers have a duplicated section number "8)"
around the _call_backend function; update the comment numbering to be sequential
and unique (e.g., change this section to "8)" and the following "8) MULTIPROCESS
\"spawn\" CONTEXT" to "9) MULTIPROCESS \"spawn\" CONTEXT" or vice versa) so
section labels are not duplicated; locate the section header string containing
"_call_backend" and the next header text "MULTIPROCESS \"spawn\" CONTEXT" and
adjust the numeral to maintain proper ordering.
- Around line 41-51: Remove the redundant type-only import of Callable inside
the TYPE_CHECKING block: since Callable is already imported at runtime (from
collections.abc import Callable), delete the duplicate "from collections.abc
import Callable" line inside the if TYPE_CHECKING: block so Callable is only
imported once and type checks still work; verify no other TYPE_CHECKING-only
imports rely on that duplicate before committing.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request fixes issues related to each trajectory duplicating unneeded information which would need to memory usage exploding for heavy simulations.
Checklist: