Skip to content

Fix qasm3 experimental exporter delay units for ms and ps#16233

Open
gkneighb wants to merge 1 commit into
Qiskit:mainfrom
gkneighb:fix-qasm3-experimental-delay-units
Open

Fix qasm3 experimental exporter delay units for ms and ps#16233
gkneighb wants to merge 1 commit into
Qiskit:mainfrom
gkneighb:fix-qasm3-experimental-delay-units

Conversation

@gkneighb
Copy link
Copy Markdown

Summary

Fixes two independent bugs in qiskit.qasm3.dumps_experimental (and dump_experimental) where Delay instructions were serialised with the wrong unit or numeric value.

Fixes #16097.

What was wrong

  1. ms was emitted as uscrates/qasm3/src/ast.rs's Display impl for DurationUnit mapped Millisecond to "us" (copy-paste from the previous arm). The emitted duration was 1000x smaller than intended.

  2. ps → ns conversion was invertedcrates/qasm3/src/exporter.rs multiplied by 1000 instead of dividing. Since 1 ps = 0.001 ns, Delay(1, 'ps') was emitted as delay[1000ns], a factor of 10^6 too large.

OpenQASM 3 has no ps literal, so the exporter must convert to ns — that part is correct; only the conversion factor was wrong.

The non-experimental Python exporter (Exporter.dumps) was unaffected and already covered by TestCircuitQASM3.test_delay_statement.

Reproduction (from the issue)

from qiskit import QuantumCircuit, qasm3
import warnings

def emit(dur, unit):
    qc = QuantumCircuit(1)
    qc.delay(dur, 0, unit=unit)
    with warnings.catch_warnings():
        warnings.simplefilter("ignore")
        out = qasm3.dumps_experimental(qc)
    line = next(l for l in out.splitlines() if "delay" in l)
    print(f"Delay({dur}, '{unit}') => {line!r}")

emit(1, "ms")   # before: 'delay[1us] q[0];'    after: 'delay[1ms] q[0];'
emit(1, "ps")   # before: 'delay[1000ns] q[0];' after: 'delay[0.001ns] q[0];'

Test plan

  • New TestQASM3ExporterRust.test_delay_units exercises every unit (ns, us, ms, s, dt, ps) with a subTest per case.
  • Existing TestQASM3ExporterRust class (19 tests) still passes.
  • Full test/python/qasm3/ suite (127 passed, 8 skipped) still passes.
  • cargo check, cargo clippy -p qiskit-qasm3 -- -D warnings, and cargo fmt --check are clean.
  • ruff check and black --check on test_export.py are clean.

AI tool disclosure

Per the Qiskit contribution policy: this change was prepared with assistance from Claude Opus 4.7 (1M context, via Claude Code). I reviewed every diff and verified all results locally; the model did not introduce any change I did not validate.

(Noting also that the originating issue #16097 was filed by @ihincks in collaboration with Claude 4.6.)

Two independent bugs caused `qasm3.dumps_experimental` to produce
incorrect output for `Delay` instructions with `unit="ms"` or
`unit="ps"`:

* `DurationUnit::Millisecond` was serialised with the suffix `"us"`
  instead of `"ms"` (a copy-paste in `ast.rs`), making the emitted
  duration read as 1000x smaller than intended.

* The picosecond-to-nanosecond conversion in `exporter.rs` multiplied
  by 1000 instead of dividing.  Since 1 ps = 0.001 ns, `Delay(1, 'ps')`
  was emitted as `delay[1000ns]` rather than `delay[0.001ns]`, a factor
  of 10^6 too large.

The existing Python exporter (`Exporter.dumps`) was already correct;
only the Rust-backed experimental path was affected.

Adds `TestQASM3ExporterRust.test_delay_units` covering every unit
(`ns`, `us`, `ms`, `s`, `dt`, `ps`).

Fixes Qiskit#16097
@gkneighb gkneighb requested a review from a team as a code owner May 22, 2026 03:04
@gkneighb gkneighb requested a review from gadial May 22, 2026 03:04
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label May 22, 2026
@qiskit-bot
Copy link
Copy Markdown
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 22, 2026

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PR PRs from contributors that are not 'members' of the Qiskit repo

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

qasm3.dumps_experimental gets some delay units wrong

3 participants