Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preliminary support for outputting to OpenQASM 3.0 #6795

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dstrain115
Copy link
Collaborator

  • This supports changing the version and the obvious changes in outputting to OpenQASM 3.0
  • This PR does not cover imports from OpenQASM 3.0

- This supports changing the version and the obvious
changes in outputting to OpenQASM 3.0
- This PR does not cover imports from OpenQASM 3.0
@dstrain115 dstrain115 requested review from vtomole and a team as code owners November 15, 2024 23:42
@CirqBot CirqBot added the size: M 50< lines changed <250 label Nov 15, 2024
Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

Nice. Please consider using a version argument in public Circuit.to_qasm instead of QasmArgs, see the inline comment.

PS: Another patch to fix failing version test -

diff --git a/cirq-core/cirq/circuits/qasm_output_test.py b/cirq-core/cirq/circuits/qasm_output_test.py
index ad4e0d44..14abacc9 100644
--- a/cirq-core/cirq/circuits/qasm_output_test.py
+++ b/cirq-core/cirq/circuits/qasm_output_test.py
@@ -225,7 +225,7 @@ rx(pi*0.123) q[0];
 def test_version():
     (q0,) = _make_qubits(1)
     with pytest.raises(ValueError):
-        output = cirq.QasmOutput((), (q0,), version='3.0')
+        output = cirq.QasmOutput((), (q0,), version='4.0')
         _ = str(output)

if comment is None:
output(f'creg {meas_id}[{len(meas.qubits)}];\n')
if self.meas_comments[key] is not None:
comment = f' // Measurement: self.meas_comments[key]'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
comment = f' // Measurement: self.meas_comments[key]'
comment = f' // Measurement: {self.meas_comments[key]}'

@@ -1362,8 +1364,10 @@ def to_qasm(
qubit_order: Determines how qubits are ordered in the QASM
register.
"""
output_precision = args.precision if args else precision
Copy link
Collaborator

@pavoljuhas pavoljuhas Nov 16, 2024

Choose a reason for hiding this comment

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

This overwrites the precision argument. Perhaps we could add a new version argument here instead and change the _qasm_ function to use _to_qasm_output instead of to_qasm.

This patch - circuit-to_qasm-with-version.patch.txt - does so and seems to pass the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants