Implementation of the Predictor Tracer#641
Merged
flowerthrower merged 32 commits intoApr 21, 2026
Merged
Conversation
- renamed CompilationVisualizer to CompilationTracer - included first draft of actual tracing logic inside predictorenv.py - adjusted qcompile.py and predictor.py accordingly to route through the tracer_output_path
The DeviceMetadata class now also includes the device's native gates, its topology and gate calibration data.
- each compilation step is now associated with its figure of merit value and its kind (exact vs. approximate)
…on steps - gate count does now not include barriers - program_communication, critical_depth, entanglement_ratio, parallelism, liveness are now also captured for each compilation step
- initial circuit is now captured as first value in the steps array (step_index 0), removed InputCircuitMetadata class from tracer.py - predictorenv.py was adjusted to calculate the initial figure_of_merit value already in the reset() method, stripping away the None-check inside the step method (step() can only ever be called after reset() was called on the environment, thereby the prev_reward and prev_reward_kind always have to be set) - changed data type for prev_reward and prev_reward_kind to float and str, respectively, and provided initial values.
The tracer now tracks the MDP policy as well as the MDP state evolution throughout the compilation.
This happens regardless of what the actual figure of merit is
… into thesis-predictor-visualization # Conflicts: # src/mqt/predictor/rl/predictor.py # src/mqt/predictor/rl/predictorenv.py
…into thesis-predictor-visualization
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a CompilationTracer that records device metadata and per-step RL compilation snapshots (metrics, features, flags) and exports the collected episode trace to a JSON file when an episode completes; exposes tracer output path through public qcompile/rl_compile/Predictor APIs and includes a test and changelog entry. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant QCompile as qcompile
participant RLCompile as rl_compile
participant Predictor as Predictor
participant Env as PredictorEnv
participant Tracer as CompilationTracer
participant FS as FileSystem
User->>QCompile: qcompile(..., tracer_output_path)
QCompile->>RLCompile: rl_compile(..., tracer_output_path)
RLCompile->>Predictor: new or existing Predictor(tracer_output_path)
Predictor->>Env: PredictorEnv(tracer_output_path)
Env->>Tracer: CompilationTracer.from_initial_state(device, circuit, fom, policy)
loop compilation steps
RLCompile->>Env: step(action)
Env->>Tracer: record_step(index, action, reward, current_qc, fom, features, flags)
end
Env->>Tracer: save_to_json() [on done]
Tracer->>FS: write JSON trace
FS-->>User: trace file persisted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mqt/predictor/rl/actions.py (1)
365-379: 🧹 Nitpick | 🔵 TrivialRemove dormant BQSKit blocks instead of retaining large commented code.
These blocks are effectively dead code and now reference removed dependencies/imports, which makes future reactivation error-prone. Prefer deleting them and keeping a brief note (or issue link) that BQSKit actions are intentionally disabled.
Also applies to: 525-547, 560-577
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mqt/predictor/rl/actions.py` around lines 365 - 379, Remove the dormant, commented BQSKit action blocks (the commented register_action/DeviceDependentAction entries referencing "BQSKitO2" and the bqskit_compile lambda) rather than keeping large commented code; delete those commented sections wherever they appear and replace each with a concise single-line note (or an issue/PR link) stating that BQSKit actions are intentionally disabled and why, so reactivation won't reference removed imports or cause confusion; ensure no other code references the symbols register_action, DeviceDependentAction, or bqskit_compile remain expecting those commented blocks.src/mqt/predictor/rl/predictor.py (1)
43-54: 🧹 Nitpick | 🔵 TrivialAdd docstring documentation for
tracer_output_pathparameter.The
Predictor.__init__docstring (line 45: "Initializes the Predictor object.") doesn't document the newtracer_output_pathparameter. Per coding guidelines, Google-style docstrings should document all parameters.Proposed docstring update
def __init__( self, figure_of_merit: figure_of_merit, device: Target, mdp: str = "paper", path_training_circuits: Path | None = None, logger_level: int = logging.INFO, tracer_output_path: str | Path | None = None, ) -> None: - """Initializes the Predictor object.""" + """Initializes the Predictor object. + + Args: + figure_of_merit: The figure of merit to be used for compilation. + device: The target device to compile to. + mdp: The MDP transition policy. Defaults to "paper". + path_training_circuits: Path to training circuits folder. Defaults to None. + logger_level: Logging level. Defaults to logging.INFO. + tracer_output_path: If provided, enables compiler tracing and exports the JSON log to this path. + """As per coding guidelines: "MUST use Google-style docstrings in Python code"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mqt/predictor/rl/predictor.py` around lines 43 - 54, The Predictor.__init__ docstring is missing documentation for the new tracer_output_path parameter; update the Google-style docstring for Predictor.__init__ to add a "Args:" entry describing tracer_output_path (type: str | Path | None, purpose: path to write tracer output or None to disable tracing) alongside the existing parameters, and ensure the wording matches the style used for the other args and mentions default behavior; no functional code changes required, just extend the docstring near the __init__ signature and keep consistent formatting with other Google-style docstrings in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 17: The changelog entry for "CompilationTracer" contains a typo ("exports
collects" → "collects") and is missing the required PR number reference; update
the entry text for CompilationTracer to read "...collects compilation
information and exports it to a JSON file" and append the PR reference in the
changelog's link references (add [`#641`]:
https://github.com/munich-quantum-toolkit/predictor/pull/641) so the entry
follows the project's template and includes the PR number.
In `@src/mqt/predictor/rl/predictor.py`:
- Around line 208-210: The code mutates
predictor_singleton.env.tracer_output_path which can leak across invocations;
instead, capture the original value, set predictor.env.tracer_output_path to the
requested tracer_output_path, invoke the compilation/reset logic (e.g. call
reset() / rl_compile on predictor), and then restore the original
tracer_output_path in a finally block so the singleton state is always reverted
(or explicitly set back to None) even on exceptions; reference
predictor_singleton, predictor.env.tracer_output_path, rl_compile, and reset()
to locate the change.
In `@src/mqt/predictor/rl/tracer.py`:
- Line 19: The import in tracer.py uses "import qiskit.qasm2 as qasm2"; update
it to the explicit module import recommended by Ruff PLR0402 by changing it to
"from qiskit import qasm2" so usages of qasm2 in this file (e.g., any calls to
qasm2.loads or qasm2.parse) keep working with the preferred import style.
- Line 224: The tracer currently calls qasm2.dumps(current_qc) directly which
can raise and abort tracing; add a static helper like _safe_qasm_dumps(qc:
QuantumCircuit) that wraps qasm2.dumps in a try/except and returns a readable
fallback string (e.g. "// QASM2 export failed: {e}") on exception, then replace
the direct call in the trace construction (where
circuit_qasm=qasm2.dumps(current_qc)) with a call to
_safe_qasm_dumps(current_qc) so failures are captured but tracing continues.
In `@tests/compilation/test_tracer.py`:
- Around line 44-46: The test currently only asserts the first compilation step
is "Baseline"; update the assertions in tests/compilation/test_tracer.py to
cover multi-step scenarios by asserting len(trace_data["steps"]) > 1 and
verifying that the final or at least one subsequent step has is_terminal == True
(e.g., assert trace_data["steps"][-1]["is_terminal"] is True or assert
any(s.get("is_terminal") for s in trace_data["steps"])). Keep the existing
checks (first action and schema_version) and add these new assertions to ensure
later steps are recorded.
- Around line 34-35: The code redundantly wraps the already-Path object
trace_file with Path(trace_file) before opening; update the call in
tests/compilation/test_tracer.py to use trace_file.open(encoding="utf-8") (or
read_text() then json.loads) instead of Path(trace_file).open(...) so the
redundant Path() wrapper is removed and the file is opened directly from the
existing trace_file Path object.
---
Outside diff comments:
In `@src/mqt/predictor/rl/actions.py`:
- Around line 365-379: Remove the dormant, commented BQSKit action blocks (the
commented register_action/DeviceDependentAction entries referencing "BQSKitO2"
and the bqskit_compile lambda) rather than keeping large commented code; delete
those commented sections wherever they appear and replace each with a concise
single-line note (or an issue/PR link) stating that BQSKit actions are
intentionally disabled and why, so reactivation won't reference removed imports
or cause confusion; ensure no other code references the symbols register_action,
DeviceDependentAction, or bqskit_compile remain expecting those commented
blocks.
In `@src/mqt/predictor/rl/predictor.py`:
- Around line 43-54: The Predictor.__init__ docstring is missing documentation
for the new tracer_output_path parameter; update the Google-style docstring for
Predictor.__init__ to add a "Args:" entry describing tracer_output_path (type:
str | Path | None, purpose: path to write tracer output or None to disable
tracing) alongside the existing parameters, and ensure the wording matches the
style used for the other args and mentions default behavior; no functional code
changes required, just extend the docstring near the __init__ signature and keep
consistent formatting with other Google-style docstrings in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8286a1a2-ca5b-4582-9e57-4bc320f4f5e8
📒 Files selected for processing (7)
CHANGELOG.mdsrc/mqt/predictor/qcompile.pysrc/mqt/predictor/rl/actions.pysrc/mqt/predictor/rl/predictor.pysrc/mqt/predictor/rl/predictorenv.pysrc/mqt/predictor/rl/tracer.pytests/compilation/test_tracer.py
wording fix Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Linus <42340116+linus-hologram@users.noreply.github.com>
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mqt/predictor/rl/predictor.py (1)
185-193:⚠️ Potential issue | 🟡 MinorUse a Google-style docstring for the new public parameter.
rl_compile()still documents parameters underArguments:. Please switch this updated API docstring to Google style (Args:/Returns:/Raises:) and keeptracer_output_pathdocumented there.As per coding guidelines: "
**/*.py: MUST use Google-style docstrings in Python code"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mqt/predictor/rl/predictor.py` around lines 185 - 193, The rl_compile docstring currently uses an "Arguments:" block; change it to Google-style docstring sections by replacing "Arguments:" with "Args:" and converting the parameter descriptions into Google-style parameter lines (e.g., qc:, device:, figure_of_merit:, predictor_singleton:, tracer_output_path:), add a "Returns:" section describing the function's return value and a "Raises:" section if applicable, and ensure tracer_output_path is documented under Args exactly as the new public parameter; update the top summary and any examples to remain consistent with the rl_compile function/method name and signatures.
♻️ Duplicate comments (3)
CHANGELOG.md (1)
17-17:⚠️ Potential issue | 🟡 MinorAdd the missing
[#641]link definition.Line 17 now references
[#641], but the PR links block still does not define that reference, so the changelog renders an unresolved link. Please add the[#641]: …/pull/641entry alongside the other PR links.Based on learnings: "In the munich-quantum-toolkit/yaqs project, changelog entries follow the template: '- $TITLE (#$NUMBER) (AUTHOR)'. Issue references should not be included in changelog entries; the PR number is sufficient for traceability."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 17, Add a missing PR link definition for the reference `[`#641`]` used in CHANGELOG.md: open the PR links block near the other pull-request link definitions and add an entry for `[`#641`]` pointing to the repository pull URL (e.g., `https://github.com/<owner>/<repo>/pull/641`) using the same link-definition format as the other entries so the `[`#641`]` reference resolves correctly.src/mqt/predictor/rl/predictor.py (1)
208-216:⚠️ Potential issue | 🟠 MajorRestore the singleton tracer path in a
finallyblock.If
compile_as_predicted()raises, Line 210's mutation survives and contaminates the shared predictor for the next call. The current cleanup also overwrites any pre-existing tracer path by forcingNoneon success. Capture the original value and restore it infinally.Suggested fix
else: predictor = predictor_singleton - predictor.env.tracer_output_path = tracer_output_path - - result = predictor.compile_as_predicted(qc) - - # Reset tracer path to prevent leakage to subsequent calls - if predictor_singleton is not None: - predictor.env.tracer_output_path = None - - return result + original_tracer_output_path = predictor.env.tracer_output_path + predictor.env.tracer_output_path = tracer_output_path + try: + return predictor.compile_as_predicted(qc) + finally: + predictor.env.tracer_output_path = original_tracer_output_path + + return predictor.compile_as_predicted(qc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/mqt/predictor/rl/predictor.py` around lines 208 - 216, The code mutates predictor.env.tracer_output_path before calling predictor.compile_as_predicted(qc) without safe restoration on exception and currently forces None on success; capture the original value (orig = predictor.env.tracer_output_path) before assigning tracer_output_path, then perform the call inside a try block and restore predictor.env.tracer_output_path = orig in a finally block (guarding any restores behind predictor_singleton/predictor existence) so the singleton’s previous tracer path is preserved whether compile_as_predicted succeeds or raises.tests/compilation/test_tracer.py (1)
47-49:⚠️ Potential issue | 🟡 MinorAssert that the trace reaches a terminal step.
These checks still pass if only the initial baseline snapshot is serialized. Please add an assertion for at least one later step and verify that the final recorded step is terminal so this test covers full-episode export, not just tracer initialization.
Suggested assertion update
assert len(trace_data["steps"]) > 0, "Tracer did not record any compilation steps." assert trace_data["steps"][0]["action"] == "Baseline" assert trace_data["schema_version"] == "1.0.0" + assert len(trace_data["steps"]) > 1, "Tracer did not record any post-baseline steps." + assert trace_data["steps"][-1]["is_terminal"] is True🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/compilation/test_tracer.py` around lines 47 - 49, The test only verifies the baseline snapshot; update it to assert the trace contains more than one step (e.g., assert len(trace_data["steps"]) > 1) and then assert the final step is terminal by checking the last step object (trace_data["steps"][-1]) has either a terminal flag set (step["terminal"] is True) or an action indicating termination (step["action"] == "Terminal"); add these two assertions alongside the existing checks for trace_data and schema_version.
🤖 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/predictor/rl/tracer.py`:
- Around line 237-244: The save_to_json method assumes the parent directory
exists; modify save_to_json (function save_to_json in tracer.py) to ensure the
output directory is created before opening the file: resolve filepath to a Path,
call .parent.mkdir(parents=True, exist_ok=True) (but avoid creating a parent for
root/single-file cases) and then proceed to open the file and
json.dump(asdict(self), ...). This ensures paths like
"artifacts/traces/run.json" won't raise FileNotFoundError.
---
Outside diff comments:
In `@src/mqt/predictor/rl/predictor.py`:
- Around line 185-193: The rl_compile docstring currently uses an "Arguments:"
block; change it to Google-style docstring sections by replacing "Arguments:"
with "Args:" and converting the parameter descriptions into Google-style
parameter lines (e.g., qc:, device:, figure_of_merit:, predictor_singleton:,
tracer_output_path:), add a "Returns:" section describing the function's return
value and a "Raises:" section if applicable, and ensure tracer_output_path is
documented under Args exactly as the new public parameter; update the top
summary and any examples to remain consistent with the rl_compile
function/method name and signatures.
---
Duplicate comments:
In `@CHANGELOG.md`:
- Line 17: Add a missing PR link definition for the reference `[`#641`]` used in
CHANGELOG.md: open the PR links block near the other pull-request link
definitions and add an entry for `[`#641`]` pointing to the repository pull URL
(e.g., `https://github.com/<owner>/<repo>/pull/641`) using the same
link-definition format as the other entries so the `[`#641`]` reference resolves
correctly.
In `@src/mqt/predictor/rl/predictor.py`:
- Around line 208-216: The code mutates predictor.env.tracer_output_path before
calling predictor.compile_as_predicted(qc) without safe restoration on exception
and currently forces None on success; capture the original value (orig =
predictor.env.tracer_output_path) before assigning tracer_output_path, then
perform the call inside a try block and restore predictor.env.tracer_output_path
= orig in a finally block (guarding any restores behind
predictor_singleton/predictor existence) so the singleton’s previous tracer path
is preserved whether compile_as_predicted succeeds or raises.
In `@tests/compilation/test_tracer.py`:
- Around line 47-49: The test only verifies the baseline snapshot; update it to
assert the trace contains more than one step (e.g., assert
len(trace_data["steps"]) > 1) and then assert the final step is terminal by
checking the last step object (trace_data["steps"][-1]) has either a terminal
flag set (step["terminal"] is True) or an action indicating termination
(step["action"] == "Terminal"); add these two assertions alongside the existing
checks for trace_data and schema_version.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b7e76b5-f069-431f-8d6a-6a120d6357f3
📒 Files selected for processing (4)
CHANGELOG.mdsrc/mqt/predictor/rl/predictor.pysrc/mqt/predictor/rl/tracer.pytests/compilation/test_tracer.py
- now also checks semantics of the first and last compilation steps - now also checks structural integrity of the compilation steps
Author
|
@CodeRabbit review |
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/compilation/test_tracer.py`:
- Around line 29-30: Update the docstring for the test function
test_compilation_tracer_generates_valid_json to follow Google-style formatting:
replace the one-line summary string with a multi-line docstring containing a
short description and an Args: section describing the tmp_path parameter (type
Path) and a Returns: section if appropriate (or omit if None), ensuring proper
indentation and blank line separation per Google-style conventions.
- Around line 34-45: You declared figure_of_merit = "expected_fidelity" but
still pass the literal "expected_fidelity" into Predictor(...) and
qcompile(...); update those call sites to use the figure_of_merit variable
instead (i.e., Predictor(figure_of_merit=figure_of_merit, ...) and qcompile(...,
figure_of_merit=figure_of_merit, ...)) so model training and compilation use the
same variable-driven value (refer to Predictor, qcompile, model_name,
model_path, and trace_file to locate the calls).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88e76872-f756-4be9-b3cf-a9541f77d9f2
📒 Files selected for processing (1)
tests/compilation/test_tracer.py
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
flowerthrower
requested changes
Apr 15, 2026
Collaborator
flowerthrower
left a comment
flowerthrower
left a comment
There was a problem hiding this comment.
Dear @linus-hologram, this looks really nice and clean already. I left a few minor comments. Should be easy to implement. Thank you for this nice work!
- separate method inside predictorenv.py that handles collection of data for tracing - qasm3 circuit strings are now used instead of qasm2
- tracer now collects all possible figures of merit - included cleaner methods for calculating the supported figures of merit and a draft for a rewrite of the calculate_reward function - included according tests
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/predictor/rl/predictorenv.py`:
- Around line 648-706: Remove the dead commented-out calculate_reward_new block
(the MARK header and the commented method body) from predictorenv.py; instead,
either add a single-line TODO with a ticket/issue reference or create a GitHub
issue to track a future refactor of calculate_reward, and ensure existing reward
dispatch is implemented by delegating to calculate_expected_fidelity,
calculate_estimated_success_probability, calculate_critical_depth, and
calculate_estimated_hellinger_distance as appropriate in a separate PR.
- Around line 456-470: The logs are using a hard-coded 0.0 instead of the
computed reward_val; update the two _log_step_reward calls to pass the computed
reward_val (the variable set by calculate_reward) rather than 0.0 — one inside
the reward_function == "estimated_hellinger_distance" branch (where reward_val
is set from calculate_reward) and the other in the main branch where reward_val
is computed before calling _collect_tracer_data; adjust both
_log_step_reward(...) calls to use reward_val so logged/terminal rewards match
what is passed to _collect_tracer_data.
- Around line 304-376: Remove the unnecessary trailing `return` at the end of
_collect_tracer_data (it returns None implicitly) and instead initialize/cache
the Hellinger model once in __init__ when tracing is enabled (tracer_output_path
is not None) and reward_function != "estimated_hellinger_distance" rather than
re-loading it inside reset(); move the joblib.load call currently in reset()
into __init__, store the model on self.hellinger_model, and update reset() to
use the cached self.hellinger_model (refer to methods _collect_tracer_data,
reset, __init__ and symbol joblib.load / self.hellinger_model).
- Around line 441-452: The code path that handles `_apply_and_update` returning
None currently forces done=True and calls `_collect_tracer_data`, which triggers
`tracer.save_to_json` and makes aborted episodes indistinguishable from normal
terminations; update this by (a) adding an explicit status flag (e.g.,
status="aborted" or aborted=True) to `_collect_tracer_data` and its downstream
`tracer.save_to_json` call so aborted traces are labeled, or (b) gate the call
to `tracer.save_to_json` inside `_collect_tracer_data` (or where
`tracer.save_to_json` is invoked) to only run when `action_name` equals the
explicit terminate action (e.g., "terminate"), and ensure the return path from
this `_apply_and_update`-None branch still returns the observation and done flag
but does not produce a saved JSON that looks like a successful terminal episode;
modify `_apply_and_update`, `_collect_tracer_data`, and the tracer save logic
accordingly to propagate the new status flag or conditional save behavior.
In `@src/mqt/predictor/rl/tracer.py`:
- Around line 28-38: The TopologyEdge dataclass currently uses misleading field
names control and target; rename these fields to neutral names (e.g., source and
target or source and destination) in the TopologyEdge class and update all
references to TopologyEdge.control and TopologyEdge.target (including any JSON
serialization/deserialization, schema producers/consumers, and unit tests) to
use the new names (TopologyEdge.source and TopologyEdge.target). Ensure any code
that constructs or parses topology JSON is updated to the new field names and
that saved schemas are versioned or documented to reflect the change.
- Around line 294-299: _extract_float currently only handles ints and
subscriptable arrays and will raise on Python floats or 0-d NumPy scalars;
change it to normalize the input with np.asarray(val) and then return a float
using .item() for single-element scalars or .ravel()[0] (or reshape(-1)[0]) for
arrays with >=1 elements, removing the isinstance(val, int) branch; also ensure
numpy is imported at runtime (not only under TYPE_CHECKING) so np.asarray is
available.
- Around line 263-292: The DeviceMetadata.description field is typed as str but
Target.description can be Optional[str]; update _extract_device_metadata to
handle None by providing a fallback string (e.g., empty string or a default like
"unknown device") when assigning device.description to
DeviceMetadata.description; modify the return construction in
_extract_device_metadata so DeviceMetadata(description=...) uses
device.description if present else the chosen default to satisfy the type
contract.
In `@tests/compilation/test_tracer.py`:
- Around line 86-97: DeviceMetadata(**trace_data["device"]) and
CompilationStep(**trace_data["steps"][0]) only catch missing/unexpected keys via
kwargs coercion but won't detect when the dataclass adds a new field with a
default (old JSON missing that key will still validate); update the test to
explicitly assert that the JSON object's key sets exactly match the dataclass
field names: derive expected fields via dataclasses.fields(DeviceMetadata) and
dataclasses.fields(CompilationStep) and compare them against
trace_data["device"].keys() and trace_data["steps"][i].keys() (also check
last_step_data), failing the test on any missing or extra keys, or alternatively
generate/validate against a JSON Schema produced from those dataclasses to
enforce strict contract parity.
- Around line 83-84: The failing assertion is order-dependent because
CompilationTracer._collect_tracer_data sets self.hellinger_model in reset() when
get_hellinger_model_path(self.device).is_file(); update the test in
tests/compilation/test_tracer.py to avoid relying on absence of a persisted
model: either (A) replace the strict assert fom_data.get("hellinger_distance")
is None with a structural check such as "hellinger_distance" in fom_data and, if
present, validate its keys/value types (e.g., contains 'value' and 'kind'), or
(B) before exercising CompilationTracer.reset()/_collect_tracer_data explicitly
remove or move get_hellinger_model_path(device) so the model file cannot exist,
ensuring deterministic fallback to None; reference
CompilationTracer._collect_tracer_data, CompilationTracer.reset, and
get_hellinger_model_path(device) when making the change.
- Around line 37-48: The variable figure_of_merit is currently untyped (inferred
as str) but is passed to Predictor and rl_compile which expect a Literal value;
fix by annotating it as a Literal literal (e.g., change the binding to:
figure_of_merit: typing.Literal["expected_fidelity"] = "expected_fidelity") or
import and use the repository alias from ty if available, ensuring the narrow
type is used where figure_of_merit is passed to Predictor and rl_compile so the
type-checker accepts the call sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0b91a29a-9740-4cd0-97d9-5cdfb6d6c1d3
📒 Files selected for processing (3)
src/mqt/predictor/rl/predictorenv.pysrc/mqt/predictor/rl/tracer.pytests/compilation/test_tracer.py
linus-hologram
commented
Apr 16, 2026
linus-hologram
commented
Apr 16, 2026
flowerthrower
requested changes
Apr 18, 2026
Collaborator
flowerthrower
left a comment
flowerthrower
left a comment
There was a problem hiding this comment.
Thanks for the continued work. Eine letzte Runde und dann sollte es passen.
…eedback incorporation
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/predictor/rl/predictorenv.py`:
- Around line 455-469: When reward_function == "estimated_hellinger_distance"
compute the current Hellinger value each step by calling
calculate_reward(mode="exact") (do not force non-terminal to 0.0), derive the
shaped step reward as the delta from prev_reward (or use terminal semantics if
done), update prev_reward and prev_reward_kind with the current value/kind,
avoid mutating self.state._layout (export a copy of layout metadata if needed
for observation/tracing), then call _log_step_reward and _collect_tracer_data
passing the actual current Hellinger FOM (fom_value) and correct fom_kind
instead of serializing 0.0.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f36fc7cd-23a2-4e0d-a442-e9e2d1f009f7
📒 Files selected for processing (2)
src/mqt/predictor/rl/predictorenv.pytests/compilation/test_tracer.py
Collaborator
|
LGTM! |
flowerthrower
merged commit e6f922e
into
munich-quantum-toolkit:qce-experiments
8 of 13 checks passed
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
2 participants
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 PR implements the Predictor Tracer backend which collects information about the RL compilation process of the MQT Predictor. If active, the tracer outputs the collected information to a json file, which will later be used in the Predictor Tracer frontend do visualize the compilation.
AI Usage
The idea, logical structure and functionality of the code are entirely my own work; Google Gemini Pro 3.1 was solely used to assist with writing valid Python code. All code that was produced by Gemini was carefully reviewed and tested by myself.
Checklist
If PR contains AI-assisted content: