feat(python): load visual circuits directly in Context#3291
Conversation
|
@microsoft-github-policy-service agree |
|
Hi @billti @idavis @minestarks, I have signed the CLA. Could you please approve the pending workflows and review when convenient? Thanks! |
Change-Id: I3b8a9eadb78dbe1f61b1ebb058a78bc1b0985a29
f084b04 to
e70656b
Compare
Change-Id: Id6234644a0f42eab2b6e6472df0464521947c71a
|
I addressed the GitHub Advanced Security / DevSkim review in |
|
All refreshed checks are passing now, including Could you please review this when convenient? Thanks! |
|
Thank you @tzh476 ! I'm taking a look now - will post comments shortly. |
minestarks
left a comment
There was a problem hiding this comment.
Really nice contribution, thank you! Just a couple of requests, please.
| if getattr(struct_type, "_qdk_context") is not self: | ||
| raise QSharpError("This struct belongs to a different Context. ") | ||
|
|
||
| def load_circuit(self, path: str, *, index: int = 0) -> Callable: |
There was a problem hiding this comment.
After discussing this within the team, we landed on import_circuit as a better name for the public API. (It mirrors import_openqasm , which has more or less the same semantics). Would you mind renaming it?
| raise QSharpError("This struct belongs to a different Context. ") | ||
|
|
||
| def load_circuit(self, path: str, *, index: int = 0) -> Callable: | ||
| """ |
There was a problem hiding this comment.
Also after team discussion: it would be valuable for this public API to take a program_type parameter (ProgramType.File | ProgramType.Operation) to mirror how import_openqasm treats its input. The ProgramType enum should be accessible here via ._native
-
File (current behavior): treats the circuit as a standalone program, the returned callable takes no arguments and can be invoked directly from Python.
-
Operation: would skip the wrapper and register the circuit operation itself (which takes a
Qubit[]parameter). This is useful for composing circuits inside larger Q# programs. However, since qubits have no Python representation, the callable could only be invoked via a string entry expression (e.g.ctx.run("{ use qs = Qubit[N]; myCircuit(qs) }", shots)), not viactx.code.myCircuit(...). You can add a unit test for this to validate behavior. See howimport_openqasmwithProgramType.Operationworks for the precedent.
The default should be File if no program type is passed.
Sorry if this seems like scope creep - when we filed the issue, we hadn't yet ironed out all open questions, but now that we see the API all fleshed out, it became clear that having both options would be useful.
Thank you!
Change-Id: I962310d4ae6aaf624aee2632d0d0f5bb7b589328
|
Thanks for the review. I pushed
Local validation: python3 -m py_compile source/qdk_package/qdk/_context.py source/qdk_package/tests/test_project.py
git diff --checkI attempted the targeted pytest locally, but this checkout cannot resolve the repository-local |
Fixes #3232.
Summary
Context.load_circuit(path, *, index=0)for standalone.qscvisual circuit files.qscfiles withindex=0by defaultContextand return a zero-argument callable suitable forctx.run(...)andctx.circuit(...)AI Assistance Disclosure
I used Codex to help inspect the existing QDK visual-circuit conversion path, draft the Python/Rust integration changes, and prepare tests. I manually reviewed the patch and verified it locally with the checks below.
Testing
test_load_circuitpassed, and all 12 tests intest_project.pypassed.