Fix scheduling units

[ElePT]

Summary

While trying to fix #11780, @Cryoris and I noticed that the rounding errors after scheduling were reporting durations of the order of e14 dt, instead of e4 dt. This was due to a conversion to dt of circuit units that were already in dt. This PR adds a fix that checks the existing units before converting.

Details and comments

To reproduce the issue, run the following code without the fix in #11780

from qiskit.circuit import QuantumCircuit
from qiskit.providers.fake_provider import GenericBackendV2
from qiskit.compiler import transpile
from qiskit.compiler.scheduler import schedule

simple = QuantumCircuit(3)
simple.sx([0, 1, 2])
simple.cx(0, 1)
simple.cx(1, 2)
simple.cx(0, 1)
simple.sx(0)

backend = GenericBackendV2(num_qubits=3, calibrate_instructions=True, seed=10)
tqc = transpile(simple, backend, scheduling_method="asap")
sched = schedule(tqc, backend)

Where the following line shows the output:

>>> sched = schedule(tqc, backend)
/Users/ept/qiskit_workspace/qiskit/qiskit/circuit/duration.py:37: UserWarning: Duration is rounded to 72234234234234 [dt] = 1.603600e+04 [s] from 1.603600e+04 [s]
  warnings.warn(

[qiskit-bot]

One or more of the the following people are requested to review this:

• @Qiskit/terra-core

[jakelishman]

Thanks for finding this. Looking at the lines above in the code, I think this might need a bit more of a tweak - would this handle the case circ.unit == "ms" correctly, for example?

Could we also add a couple of regression tests on it and a bugfix note? Do you think it's eligible for backport to 0.46 as well?

[coveralls]

Pull Request Test Coverage Report for Build 7889916990

Details

• -1 of 7 (85.71%) changed or added relevant lines in 1 file are covered.
• 5 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.03%) to 89.205%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/duration.py	6	7	85.71%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	5	91.44%

Totals
Change from base Build 7886503866:	0.03%
Covered Lines:	58830
Relevant Lines:	65949

---

💛 - Coveralls

[ElePT]

Could we also add a couple of regression tests on it and a bugfix note? Do you think it's eligible for backport to 0.46 as well?

Added. I think it's elegible for backport to both 0.45 and 0.46.

Thanks for finding this. Looking at the lines above in the code, I think this might need a bit more of a tweak - would this handle the case circ.unit == "ms" correctly, for example?

The bug was in convert_durations_to_dt(), which I understand only expects inputs in dt or s:
https://github.com/Qiskit/qiskit/blob/main/qiskit/circuit/duration.py#L46-L60. These are the only 2 cases contemplated for operations (where the units were checked), that's why I applied the same logic to the full circuit duration.

[jakelishman]

The bug was in convert_durations_to_dt(), which I understand only expects inputs in dt or s

Well, convert_durations_to_dt expects inputs in any unit (the choices are prefixed versions of seconds or dt) - for the operations in the lines above the ones touched here, it does the apply_prefix stuff, but for QuantumCircuit.duration it doesn't - if it's not already dt it assumes it's s, but I don't know of any reason that couldn't be ms. I think we ought to be doing the apply_prefix stuff on QuantumCircuit.duration, like this function does for operation.duration.

[jakelishman]

This looks fine to me, thanks. I think the bug's sufficiently non-critical (must have been around for ages, I think?) that it's best to wait to merge til we can target it at 1.0.1, rather than accepting the heightened risk of merging it between rc1 and 1.0.0 final.

I'll leave this approved but untagged until stable/1.0 is open for bugfixes for 1.0.1.