Implementation of reading direct qasm files#450
Open
Marerido wants to merge 12 commits into
Open
Conversation
Member
|
@Marerido Can you move the bulk of the implementation to something like Also, the pre-commit is a step with ruff/ty that checks the style and formatting of your code. You can run this locally as well to go through each individual error (I guess just docs for now). The tests may be failing because of an import, or it could be that the CI online doesn't have the correct imports due to the package configuration. I can look at it later if you can't figure it out. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Member
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds QASM file and string input support. It introduces a shared load_circuit() utility that detects OPENQASM version and dispatches to qasm2/qasm3, and integrates it into EquivalenceChecker.check() and Simulator.run(), with unit and integration tests (QASM3 tests gated by optional import) and an OpenQASM 3 fixture. ChangesQASM input support across library
Sequence Diagram(s)sequenceDiagram
participant Caller
participant EquivalenceChecker
participant Simulator
participant Loader as load_circuit
participant QASM2 as qasm2.loads/load
participant QASM3 as qasm3.loads/load
Caller->>EquivalenceChecker: call .check(circuit1, circuit2)
EquivalenceChecker->>Loader: load_circuit(circuit1)
Loader->>QASM2: qasm2.loads/load (if not OPENQASM 3)
Loader->>QASM3: qasm3.loads/load (if OPENQASM 3)
Loader-->>EquivalenceChecker: QuantumCircuit
EquivalenceChecker->>EquivalenceChecker: perform equivalence
Caller->>Simulator: call .run(operator)
Simulator->>Loader: load_circuit(operator)
Loader->>QASM2: qasm2.loads/load (if not OPENQASM 3)
Loader->>QASM3: qasm3.loads/load (if OPENQASM 3)
Loader-->>Simulator: QuantumCircuit
Simulator->>Simulator: route to analog or circuit simulation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mqt/yaqs/simulator.py (1)
655-657:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate
run()docstring for the expandedoperatorcontract.Line 655 still documents only
Hamiltonian/QuantumCircuit, but the signature now acceptsstr | Path(QASM input) too. Please align theArgs.operatortext with the public API.Suggested docstring update
- operator: :class:`~mqt.yaqs.core.data_structures.hamiltonian.Hamiltonian` for analog - simulations or a :class:`~qiskit.circuit.QuantumCircuit` for circuit simulations. + operator: :class:`~mqt.yaqs.core.data_structures.hamiltonian.Hamiltonian` for analog + simulations, or for circuit simulations one of: + :class:`~qiskit.circuit.QuantumCircuit`, a raw OPENQASM string, or a filesystem + path (``str``/``Path``) to a QASM file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mqt/yaqs/simulator.py` around lines 655 - 657, The docstring for run() still lists only Hamiltonian/QuantumCircuit types for the operator argument; update Args.operator in the run() docstring to reflect the expanded public API by stating it accepts mqt.yaqs.core.data_structures.hamiltonian.Hamiltonian, qiskit.circuit.QuantumCircuit, or a str | Path pointing to a QASM file (QASM input), and briefly describe how each type is interpreted (analog Hamiltonian, circuit simulation, or QASM parsed into a circuit). Make sure to reference the run() signature and use the exact symbol name operator in the Args section so callers see the supported types and behaviors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mqt/yaqs/digital/utils/qasm_utils.py`:
- Around line 21-26: Add a Google-style docstring to the _first_non_comment_line
function: describe its purpose (return the first non-empty, non-comment line
from a QASM-like text), document the argument (text: str — multiline input), and
the return value (str — the first non-comment line or empty string if none).
Place the docstring immediately below the def _first_non_comment_line(...) line
and follow Google docstring sections "Args:" and "Returns:".
- Around line 41-44: Cache the result of _first_non_comment_line(circuit)
instead of calling it twice: compute something like header =
_first_non_comment_line(circuit) once inside the branch that checks
isinstance(circuit, str), then use header.startswith("OPENQASM") for the outer
check and header.startswith("OPENQASM 3") to decide between calling
qasm3.loads(circuit) and qasm2.loads(circuit); update the code paths around
circuit, _first_non_comment_line, qasm3.loads and qasm2.loads accordingly to
reuse the cached header.
In `@tests/test_simulator.py`:
- Around line 1457-1487: Add a new unit test that passes a raw OPENQASM string
(not a file path) into Simulator.run to exercise the direct-string operator
path: read the contents of "circuit.qasm" (and optionally "circuit3.qasm"
guarded by pytest.importorskip("qiskit_qasm3_import")) into a str and call
Simulator(parallel=False, show_progress=False).run(state, qasm_string,
sim_params) using existing helpers State and WeakSimParams, then assert
result.counts is not None and sum(result.counts.values()) == sim_params.shots;
name the test clearly (e.g., test_simulator_run_accepts_qasm2_raw_string) and
mirror the setup from test_simulator_run_accepts_qasm2_str_path to keep
consistency.
- Around line 1498-1504: The test test_simulator_run_strong_accepts_qasm_string
is constructing qasm_string as a Path, so it doesn't verify passing a str path;
update the qasm_string assignment to be a plain str (for example by wrapping the
Path with str(...) or building the path as a string) so that
Simulator.run(state, qasm_string, sim_params) is actually called with a str,
keeping the rest of the test (State, StrongSimParams, Simulator.run, and the
final assertion) unchanged.
---
Outside diff comments:
In `@src/mqt/yaqs/simulator.py`:
- Around line 655-657: The docstring for run() still lists only
Hamiltonian/QuantumCircuit types for the operator argument; update Args.operator
in the run() docstring to reflect the expanded public API by stating it accepts
mqt.yaqs.core.data_structures.hamiltonian.Hamiltonian,
qiskit.circuit.QuantumCircuit, or a str | Path pointing to a QASM file (QASM
input), and briefly describe how each type is interpreted (analog Hamiltonian,
circuit simulation, or QASM parsed into a circuit). Make sure to reference the
run() signature and use the exact symbol name operator in the Args section so
callers see the supported types and behaviors.
🪄 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: 50a0fd99-3cee-4556-b31e-a1a48ab05fef
📒 Files selected for processing (7)
src/mqt/yaqs/digital/utils/qasm_utils.pysrc/mqt/yaqs/equivalence_checker.pysrc/mqt/yaqs/simulator.pytests/circuit3.qasmtests/digital/utils/test_qasm_utils.pytests/test_equivalence_checker.pytests/test_simulator.py
Comment on lines
+21
to
+26
| def _first_non_comment_line(text: str) -> str: | ||
| for line in text.splitlines(): | ||
| stripped = line.strip() | ||
| if stripped and not stripped.startswith("//"): | ||
| return stripped | ||
| return "" |
Contributor
There was a problem hiding this comment.
Add a Google-style docstring to _first_non_comment_line.
The function lacks a docstring. As per coding guidelines, all Python functions should have Google-style docstrings that describe purpose, arguments, and return values.
📝 Suggested docstring
def _first_non_comment_line(text: str) -> str:
+ """Return the first non-blank, non-comment line from QASM text.
+
+ Args:
+ text: QASM source text, potentially with leading comments.
+
+ Returns:
+ The first line that is neither blank nor a ``//`` comment,
+ or an empty string if no such line exists.
+ """
for line in text.splitlines():📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _first_non_comment_line(text: str) -> str: | |
| for line in text.splitlines(): | |
| stripped = line.strip() | |
| if stripped and not stripped.startswith("//"): | |
| return stripped | |
| return "" | |
| def _first_non_comment_line(text: str) -> str: | |
| """Return the first non-blank, non-comment line from QASM text. | |
| Args: | |
| text: QASM source text, potentially with leading comments. | |
| Returns: | |
| The first line that is neither blank nor a ``//`` comment, | |
| or an empty string if no such line exists. | |
| """ | |
| for line in text.splitlines(): | |
| stripped = line.strip() | |
| if stripped and not stripped.startswith("//"): | |
| return stripped | |
| return "" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mqt/yaqs/digital/utils/qasm_utils.py` around lines 21 - 26, Add a
Google-style docstring to the _first_non_comment_line function: describe its
purpose (return the first non-empty, non-comment line from a QASM-like text),
document the argument (text: str — multiline input), and the return value (str —
the first non-comment line or empty string if none). Place the docstring
immediately below the def _first_non_comment_line(...) line and follow Google
docstring sections "Args:" and "Returns:".
Source: Coding guidelines
Comment on lines
+41
to
+44
| if isinstance(circuit, str) and _first_non_comment_line(circuit).startswith("OPENQASM"): | ||
| if _first_non_comment_line(circuit).startswith("OPENQASM 3"): # pragma: no cover | ||
| return qasm3.loads(circuit) | ||
| return qasm2.loads(circuit) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Cache _first_non_comment_line result to avoid redundant computation.
The function is called twice when processing a QASM string input: once in the condition at line 41 and again at line 42. Store the result once and reuse it for clarity and efficiency.
♻️ Proposed refactor
if isinstance(circuit, str) and _first_non_comment_line(circuit).startswith("OPENQASM"):
- if _first_non_comment_line(circuit).startswith("OPENQASM 3"): # pragma: no cover
+ first_line = _first_non_comment_line(circuit)
+ if first_line.startswith("OPENQASM 3"): # pragma: no cover
return qasm3.loads(circuit)
return qasm2.loads(circuit)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mqt/yaqs/digital/utils/qasm_utils.py` around lines 41 - 44, Cache the
result of _first_non_comment_line(circuit) instead of calling it twice: compute
something like header = _first_non_comment_line(circuit) once inside the branch
that checks isinstance(circuit, str), then use header.startswith("OPENQASM") for
the outer check and header.startswith("OPENQASM 3") to decide between calling
qasm3.loads(circuit) and qasm2.loads(circuit); update the code paths around
circuit, _first_non_comment_line, qasm3.loads and qasm2.loads accordingly to
reuse the cached header.
Comment on lines
+1457
to
+1487
| def test_simulator_run_accepts_qasm2_str_path() -> None: | ||
| """Verify that Simulator.run accepts a QASM 2 file passed as a str path.""" | ||
| qasm_file = str(Path(__file__).parent / "circuit.qasm") | ||
| state = State(6, initial="zeros") | ||
| sim_params = WeakSimParams(shots=4, max_bond_dim=4) | ||
| result = Simulator(parallel=False, show_progress=False).run(state, qasm_file, sim_params) | ||
| assert result.counts is not None | ||
| assert sum(result.counts.values()) == sim_params.shots | ||
|
|
||
|
|
||
| def test_simulator_run_accepts_qasm3_path_object() -> None: | ||
| """Verify that Simulator.run accepts a QASM 3 file passed as a Path object.""" | ||
| pytest.importorskip("qiskit_qasm3_import") | ||
| qasm_file = Path(__file__).parent / "circuit3.qasm" | ||
| state = State(2, initial="zeros") | ||
| sim_params = WeakSimParams(shots=4, max_bond_dim=4) | ||
| result = Simulator(parallel=False, show_progress=False).run(state, qasm_file, sim_params) | ||
| assert result.counts is not None | ||
| assert sum(result.counts.values()) == sim_params.shots | ||
|
|
||
|
|
||
| def test_simulator_run_accepts_qasm3_str_path() -> None: | ||
| """Verify that Simulator.run accepts a QASM 3 file passed as a str path.""" | ||
| pytest.importorskip("qiskit_qasm3_import") | ||
| qasm_file = str(Path(__file__).parent / "circuit3.qasm") | ||
| state = State(2, initial="zeros") | ||
| sim_params = WeakSimParams(shots=4, max_bond_dim=4) | ||
| result = Simulator(parallel=False, show_progress=False).run(state, qasm_file, sim_params) | ||
| assert result.counts is not None | ||
| assert sum(result.counts.values()) == sim_params.shots | ||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add one simulator-level test for raw OPENQASM string input.
Current “str” tests cover stringified file paths. Given the feature scope, add an end-to-end Simulator.run(..., operator=<raw qasm text>, ...) assertion to protect the direct-string integration path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_simulator.py` around lines 1457 - 1487, Add a new unit test that
passes a raw OPENQASM string (not a file path) into Simulator.run to exercise
the direct-string operator path: read the contents of "circuit.qasm" (and
optionally "circuit3.qasm" guarded by
pytest.importorskip("qiskit_qasm3_import")) into a str and call
Simulator(parallel=False, show_progress=False).run(state, qasm_string,
sim_params) using existing helpers State and WeakSimParams, then assert
result.counts is not None and sum(result.counts.values()) == sim_params.shots;
name the test clearly (e.g., test_simulator_run_accepts_qasm2_raw_string) and
mirror the setup from test_simulator_run_accepts_qasm2_str_path to keep
consistency.
Comment on lines
+1498
to
+1504
| def test_simulator_run_strong_accepts_qasm_string() -> None: | ||
| """Verify that Simulator.run with StrongSimParams accepts a QASM file passed as a str path.""" | ||
| qasm_string = Path(__file__).parent / "circuit.qasm" | ||
| state = State(6, initial="zeros") | ||
| sim_params = StrongSimParams(observables=[Observable(Z(), 0)], num_traj=1, max_bond_dim=4) | ||
| result = Simulator(parallel=False, show_progress=False).run(state, qasm_string, sim_params) | ||
| assert result.expectation_values[0] is not None |
Contributor
There was a problem hiding this comment.
test_simulator_run_strong_accepts_qasm_string is passing a Path, not a str.
Line 1500 constructs a Path, so this test does not actually validate the advertised string-path case in strong mode.
Minimal fix
- qasm_string = Path(__file__).parent / "circuit.qasm"
+ qasm_string = str(Path(__file__).parent / "circuit.qasm")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_simulator.py` around lines 1498 - 1504, The test
test_simulator_run_strong_accepts_qasm_string is constructing qasm_string as a
Path, so it doesn't verify passing a str path; update the qasm_string assignment
to be a plain str (for example by wrapping the Path with str(...) or building
the path as a string) so that Simulator.run(state, qasm_string, sim_params) is
actually called with a str, keeping the rest of the test (State,
StrongSimParams, Simulator.run, and the final assertion) unchanged.
Member
|
@Marerido If you look at the "Prompt for all review comments", you can copy paste that into coding AI and it will take care of these things. They are mostly superficial documentation comments right now. Once you fix these, I'll do a review and make sure that the structure of your additions look good one last time. |
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 (1)
src/mqt/yaqs/equivalence_checker.py (1)
173-184: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winClarify
strinput semantics incheck()docstring.The signature now accepts
str | Path, but the Args text still reads like circuit-only objects. Please document thatstrcan be either a filesystem path or raw OpenQASM content to avoid caller ambiguity.[Suggesting this as API-surface documentation polish aligned with the new behavior.]
Proposed docstring update
Args: - circuit1: First quantum circuit. - circuit2: Second quantum circuit (must have the same number of qubits). + circuit1: First circuit input. Accepts a ``QuantumCircuit``, a ``Path``, or a ``str`` + containing either a file path or raw OpenQASM text. + circuit2: Second circuit input (same accepted types as ``circuit1``; must resolve to + a circuit with the same number of qubits).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mqt/yaqs/equivalence_checker.py` around lines 173 - 184, Update the check() docstring to clarify the semantics of the circuit1/circuit2 parameters: specify that when a parameter is a str it may be either a filesystem path pointing to a circuit file or raw OpenQASM text (in addition to accepting QuantumCircuit or Path), and document how the function distinguishes between the two (e.g., by attempting to read a file or parsing as OpenQASM). Reference the check() function signature and adjust the Args section to explicitly list allowed types and examples for str inputs so callers know the accepted formats.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mqt/yaqs/simulator.py`:
- Around line 671-673: The code currently calls load_circuit(operator) for any
str/Path, causing analog runs to fail early; change the logic so that
load_circuit is only invoked when the simulator is in circuit mode (e.g., check
the relevant mode/run_type flag used by the dispatch path) and leave operator
untouched for non-circuit/analog modes so later type validation can run; update
the branch around operator and load_circuit() (referencing operator and
load_circuit) to first test the simulator's mode (the same mode/flag used by the
dispatch function) and only call load_circuit when that mode indicates a
circuit/QASM run.
In `@tests/digital/utils/test_qasm_utils.py`:
- Line 18: The noqa on _first_non_comment_line lacks an explanatory
justification; update the test to add an inline comment explaining why PLC2701
is suppressed (for example: this test intentionally imports/uses a fixture or
symbol only for side effects/registration and is not referenced directly),
keeping the # noqa: PLC2701 token but adding the brief reason after it so
reviewers and linters understand the intentional suppression referencing the
_first_non_comment_line symbol.
In `@tests/test_equivalence_checker.py`:
- Around line 330-375: Add integration tests that pass raw OpenQASM content
strings into EquivalenceChecker.check(); specifically, create new tests like
test_check_accepts_qasm2_raw_string and test_check_accepts_qasm3_raw_string that
read the existing "circuit.qasm" and "circuit3.qasm" files via
Path(...).read_text() and call checker.check(qasm_str, qasm_str) (use
representation="mpo" for QASM2 and representation="matrix" for QASM3). For QASM3
test, wrap with pytest.importorskip("qiskit_qasm3_import") as done in
test_check_accepts_qasm3_path_object, and assert result["equivalent"] is True to
match the other path/string tests.
---
Outside diff comments:
In `@src/mqt/yaqs/equivalence_checker.py`:
- Around line 173-184: Update the check() docstring to clarify the semantics of
the circuit1/circuit2 parameters: specify that when a parameter is a str it may
be either a filesystem path pointing to a circuit file or raw OpenQASM text (in
addition to accepting QuantumCircuit or Path), and document how the function
distinguishes between the two (e.g., by attempting to read a file or parsing as
OpenQASM). Reference the check() function signature and adjust the Args section
to explicitly list allowed types and examples for str inputs so callers know the
accepted formats.
🪄 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: 5d8aa1bf-481a-4bde-8d5f-e39d8789a481
📒 Files selected for processing (7)
src/mqt/yaqs/digital/utils/qasm_utils.pysrc/mqt/yaqs/equivalence_checker.pysrc/mqt/yaqs/simulator.pytests/circuit3.qasmtests/digital/utils/test_qasm_utils.pytests/test_equivalence_checker.pytests/test_simulator.py
Comment on lines
+671
to
+673
| if isinstance(operator, (str, Path)): | ||
| operator = load_circuit(operator) | ||
|
|
Contributor
There was a problem hiding this comment.
Restrict QASM normalization to circuit modes only.
load_circuit() is executed before dispatch, so analog runs with operator as str/Path can fail with file/parser errors instead of the intended analog type validation.
Suggested fix
- if isinstance(operator, (str, Path)):
- operator = load_circuit(operator)
+ if isinstance(sim_params, (StrongSimParams, WeakSimParams)) and isinstance(operator, (str, Path)):
+ operator = load_circuit(operator)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mqt/yaqs/simulator.py` around lines 671 - 673, The code currently calls
load_circuit(operator) for any str/Path, causing analog runs to fail early;
change the logic so that load_circuit is only invoked when the simulator is in
circuit mode (e.g., check the relevant mode/run_type flag used by the dispatch
path) and leave operator untouched for non-circuit/analog modes so later type
validation can run; update the branch around operator and load_circuit()
(referencing operator and load_circuit) to first test the simulator's mode (the
same mode/flag used by the dispatch function) and only call load_circuit when
that mode indicates a circuit/QASM run.
| from qiskit import QuantumCircuit | ||
|
|
||
| from mqt.yaqs.digital.utils.qasm_utils import ( | ||
| _first_non_comment_line, # noqa: PLC2701 |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document why the # noqa suppression is necessary.
The suppression lacks an explanatory comment. As per coding guidelines, ignore rules should document why they are necessary.
📝 Suggested comment
from mqt.yaqs.digital.utils.qasm_utils import (
- _first_non_comment_line, # noqa: PLC2701
+ _first_non_comment_line, # noqa: PLC2701 - testing private function
load_circuit,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| _first_non_comment_line, # noqa: PLC2701 | |
| from mqt.yaqs.digital.utils.qasm_utils import ( | |
| _first_non_comment_line, # noqa: PLC2701 - testing private function | |
| load_circuit, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/digital/utils/test_qasm_utils.py` at line 18, The noqa on
_first_non_comment_line lacks an explanatory justification; update the test to
add an inline comment explaining why PLC2701 is suppressed (for example: this
test intentionally imports/uses a fixture or symbol only for side
effects/registration and is not referenced directly), keeping the # noqa:
PLC2701 token but adding the brief reason after it so reviewers and linters
understand the intentional suppression referencing the _first_non_comment_line
symbol.
Source: Coding guidelines
Comment on lines
+330
to
+375
| def test_check_accepts_qasm2_path_object() -> None: | ||
| """Check that a QASM 2 file given as a Path object is accepted and returns equivalent.""" | ||
| qasm_path = Path(__file__).parent / "circuit.qasm" | ||
|
|
||
| checker = EquivalenceChecker(representation="mpo") | ||
| result = checker.check(qasm_path, qasm_path) | ||
| assert result["equivalent"] is True | ||
|
|
||
|
|
||
| def test_check_accepts_qasm2_str_path() -> None: | ||
| """Check that a QASM 2 file given as a str path is accepted and returns equivalent.""" | ||
| qasm_path = str(Path(__file__).parent / "circuit.qasm") | ||
|
|
||
| checker = EquivalenceChecker(representation="mpo") | ||
| result = checker.check(qasm_path, qasm_path) | ||
| assert result["equivalent"] is True | ||
|
|
||
|
|
||
| def test_check_qasm_path_vs_quantumcircuit_agree() -> None: | ||
| """Verify that loading via path and via QuantumCircuit gives the same equivalence result.""" | ||
| qasm_path = Path(__file__).parent / "circuit.qasm" | ||
| qc = load(filename=str(qasm_path)) | ||
| checker = EquivalenceChecker(representation="mpo") | ||
| result_path = checker.check(qasm_path, qasm_path) | ||
| result_qc = checker.check(qc, qc) | ||
| assert result_path["equivalent"] == result_qc["equivalent"] | ||
|
|
||
|
|
||
| def test_check_accepts_qasm3_path_object() -> None: | ||
| """Check that a QASM 3 file given as a Path object is accepted and returns equivalent.""" | ||
| pytest.importorskip("qiskit_qasm3_import") | ||
| qasm_file = Path(__file__).parent / "circuit3.qasm" | ||
|
|
||
| checker = EquivalenceChecker(representation="matrix") | ||
| result = checker.check(qasm_file, qasm_file) | ||
| assert result["equivalent"] is True | ||
|
|
||
|
|
||
| def test_check_accepts_qasm3_str_path() -> None: | ||
| """Check that a QASM 3 file given as a str path is accepted and returns equivalent.""" | ||
| pytest.importorskip("qiskit_qasm3_import") | ||
| qasm_file = str(Path(__file__).parent / "circuit3.qasm") | ||
|
|
||
| checker = EquivalenceChecker(representation="matrix") | ||
| result = checker.check(qasm_file, qasm_file) | ||
| assert result["equivalent"] is True |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add integration tests for raw OpenQASM string inputs to check().
These additions verify Path and string-path handling, but the PR feature also includes direct QASM string inputs. Adding raw-QASM integration tests here will guard the public EquivalenceChecker.check() contract directly.
Proposed test additions
+def test_check_accepts_qasm2_raw_string() -> None:
+ """Check that raw OpenQASM 2 text input is accepted."""
+ qasm2 = """\
+OPENQASM 2.0;
+include "qelib1.inc";
+qreg q[1];
+h q[0];
+"""
+ checker = EquivalenceChecker(representation="matrix")
+ result = checker.check(qasm2, qasm2)
+ assert result["equivalent"] is True
+
+
+def test_check_accepts_qasm3_raw_string() -> None:
+ """Check that raw OpenQASM 3 text input is accepted."""
+ pytest.importorskip("qiskit_qasm3_import")
+ qasm3 = """\
+OPENQASM 3.0;
+include "stdgates.inc";
+qubit[1] q;
+h q[0];
+"""
+ checker = EquivalenceChecker(representation="matrix")
+ result = checker.check(qasm3, qasm3)
+ assert result["equivalent"] is True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_equivalence_checker.py` around lines 330 - 375, Add integration
tests that pass raw OpenQASM content strings into EquivalenceChecker.check();
specifically, create new tests like test_check_accepts_qasm2_raw_string and
test_check_accepts_qasm3_raw_string that read the existing "circuit.qasm" and
"circuit3.qasm" files via Path(...).read_text() and call checker.check(qasm_str,
qasm_str) (use representation="mpo" for QASM2 and representation="matrix" for
QASM3). For QASM3 test, wrap with pytest.importorskip("qiskit_qasm3_import") as
done in test_check_accepts_qasm3_path_object, and assert result["equivalent"] is
True to match the other path/string tests.
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
3 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
Please include a summary of the change and, if applicable, which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes #(issue)
Checklist
If PR contains AI-assisted content:
Assisted-by: [Model Name] via [Tool Name]footer.