Add method to compute estimated duration of scheduled circuit #13783

mtreinish · 2025-02-03T22:24:29Z

Summary

This commit adds a new QuantumCircuit method to compute the estimated duration of a scheduled circuit. This is to replace the deprecated duration attribute that the transpiler was potentially setting during the scheduling stage. This method computes the longest duration path in the dag view of the circuit internally.

This method should have been included in the 1.2.0 release prior to the deprecation of the QuantumCircuit.duration attribute in 1.3.0. But, this was an oversight in the path to deprecation, as it was part of larger deprecation of numerous scheduling pieces in the 1.3.0. We should definitely backport this for the 1.4.0 release for inclusion in that release prior to the Qiskit 2.0.0 release which removes the deprecated attribute

Details and comments

TODO:

Add tests (and fix issues as this is untested so far)

This commit adds a new QuantumCircuit method to compute the estimated duration of a scheduled circuit. This is to replace the deprecated duration attribute that the transpiler was potentially setting during the scheduling stage. This method computes the longest duration path in the dag view of the circuit internally. This method should have been included in the 1.2.0 release prior to the deprecation of the `QuantumCircuit.duration` attribute in 1.3.0. But, this was an oversight in the path to deprecation, as it was part of larger deprecation of numerous scheduling pieces in the 1.3.0. We should definitely backport this for the 1.4.0 release for inclusion in that release prior to the Qiskit 2.0.0 release which removes the deprecated attribute

mtreinish · 2025-02-03T22:25:02Z

@Mergifyio backport stable/1.4

mergify · 2025-02-03T22:25:06Z

backport stable/1.4

✅ Backports have been created

#13881 Add method to compute estimated duration of scheduled circuit (backport #13783) has been created for branch stable/1.4

coveralls · 2025-02-03T22:54:03Z

Pull Request Test Coverage Report for Build 13400631790

Details

72 of 89 (80.9%) changed or added relevant lines in 5 files are covered.
7 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.005%) to 88.109%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/accelerate/src/circuit_duration.rs	49	66	74.24%

Files with Coverage Reduction	New Missed Lines	%
crates/accelerate/src/unitary_synthesis.rs	3	94.29%
crates/qasm2/src/lex.rs	4	92.48%

Totals
Change from base Build 13397495918:	0.005%
Covered Lines:	78690
Relevant Lines:	89310

💛 - Coveralls

jakelishman

Thanks for doing this.

This feels like we probably ought to have a migration guide or something to both explain ourselves a bit more and to include a couple more worked examples to help the transition?

crates/accelerate/src/circuit_duration.rs

jakelishman · 2025-02-03T22:49:31Z

qiskit/circuit/quantumcircuit.py

-    available through the :attr:`duration`, :attr:`unit` and :attr:`op_start_times` attributes.
+    available through the :meth:`estimate_duration` method and :attr:`op_start_times` attribute.


Are we leaving op_start_times as a valid attribute of QuantumCircuit post deprecation?

I didn't think that we deprecated it as part of 1.3.0. There are also a bunch of associated methods with it too. We can deprecate it for 1.4.0 as falls into the same category as duration though and we should probably remove it from the circuit object. But we can look at that in a separate pr.

I looked into doing this and don't think we should for 2.0. The timeline visualizer depends on this data and methods. I need to figure out a more complete mechanism to rewrite the drawer without this, and rushing it in the week of 1.4.0 doesn't seem ideal. We should update the docstrings in 2.0 though to make it more explicit that this is an estimate etc, like what this method does for the duration.

I added an issue to keep track of the docstring update: #13880, it's very small but I am afraid that without it we might forget to do it

qiskit/circuit/quantumcircuit.py

jakelishman · 2025-02-03T22:54:01Z

crates/accelerate/src/circuit_duration.rs

+    let get_duration =
+        |edge: <&StableDiGraph<NodeType, Wire> as IntoEdgeReferences>::EdgeRef| -> PyResult<f64> {
+            let node_weight = &dag.dag()[edge.target()];
+            match node_weight {


The nesting's got a bit out of control in this function - I think we reach 11 levels of indentation at the deepest point? Some of the if lets might want to become let ... else or anything else we can do.

I dropped an indent level as part of refactoring for the rust standard instruction in: 0fe8e0d (I'll have to rewrite it for 1.4 backport though since StandardInstruction is only in 2.0).

1ucian0

~~compute_estimated_duration sounds like a verbose name. @nonhermitian suggested estimate_duration and I think it is much better.~~ ups.. it was already that name. my bad.

qiskit-bot · 2025-02-17T21:25:56Z

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

@Qiskit/terra-core

mtreinish · 2025-02-17T21:36:34Z

This should be ready for review now. I'll have to rewrite the rust implementation when it's backported to 1.4 because the rust internals on main are different than in 1.3, but it shouldn't be too hard it's basically the implementation before: 0fe8e0d

ElePT

LGTM! My only concern here is how to make sure that this new utility reaches users I think that at least we can make sure to include it in the 2.0 migration guide (added a comment in the issue).

* Add method to compute estimated duration of scheduled circuit This commit adds a new QuantumCircuit method to compute the estimated duration of a scheduled circuit. This is to replace the deprecated duration attribute that the transpiler was potentially setting during the scheduling stage. This method computes the longest duration path in the dag view of the circuit internally. This method should have been included in the 1.2.0 release prior to the deprecation of the `QuantumCircuit.duration` attribute in 1.3.0. But, this was an oversight in the path to deprecation, as it was part of larger deprecation of numerous scheduling pieces in the 1.3.0. We should definitely backport this for the 1.4.0 release for inclusion in that release prior to the Qiskit 2.0.0 release which removes the deprecated attribute * Simplify dag node indexing * Expand docs * Fix handling for StandardInstruction in rust and add first test * Expand test coverage * Fix lint (cherry picked from commit 8c50ce4)

In the recently merged Qiskit#13783 a bug slipped in an unexercised code path where we were dividing by dt instead of multiplying. This was fixed in the common code path where dt is an int, but nothing in the data model disallows float durations for dt units despite it not being real. Since the data model allowed it the rust code needed to support it and that path did the wrong operation. This commit fixes that oversight in the rust code and adds a test for it.

In the recently merged #13783 a bug slipped in an unexercised code path where we were dividing by dt instead of multiplying. This was fixed in the common code path where dt is an int, but nothing in the data model disallows float durations for dt units despite it not being real. Since the data model allowed it the rust code needed to support it and that path did the wrong operation. This commit fixes that oversight in the rust code and adds a test for it.

#13783) (#13881) * Add method to compute estimated duration of scheduled circuit (#13783) * Add method to compute estimated duration of scheduled circuit This commit adds a new QuantumCircuit method to compute the estimated duration of a scheduled circuit. This is to replace the deprecated duration attribute that the transpiler was potentially setting during the scheduling stage. This method computes the longest duration path in the dag view of the circuit internally. This method should have been included in the 1.2.0 release prior to the deprecation of the `QuantumCircuit.duration` attribute in 1.3.0. But, this was an oversight in the path to deprecation, as it was part of larger deprecation of numerous scheduling pieces in the 1.3.0. We should definitely backport this for the 1.4.0 release for inclusion in that release prior to the Qiskit 2.0.0 release which removes the deprecated attribute * Simplify dag node indexing * Expand docs * Fix handling for StandardInstruction in rust and add first test * Expand test coverage * Fix lint (cherry picked from commit 8c50ce4) * Update rust code to work with 1.4.0 rust data model * Fix bug in arithmetic for converting dt to sec * Add fix test * Fix lint --------- Co-authored-by: Matthew Treinish <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]> Co-authored-by: Elena Peña Tapia <[email protected]>

mtreinish added Changelog: New Feature Include in the "Added" section of the changelog Rust This PR or issue is related to Rust code in the repository mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library labels Feb 3, 2025

mtreinish added this to the 1.4.0 milestone Feb 3, 2025

mtreinish assigned ElePT Feb 3, 2025

jakelishman reviewed Feb 3, 2025

View reviewed changes

1ucian0 reviewed Feb 4, 2025

View reviewed changes

mtreinish mentioned this pull request Feb 6, 2025

Remove condition/c_if, duration, and unit from instructions #13506

Merged

5 tasks

mtreinish mentioned this pull request Feb 14, 2025

Remove deprecated QuantumCircuit.duration and DAGCircuit.duration #13849

Open

mtreinish added 5 commits February 17, 2025 14:07

Merge remote-tracking branch 'origin/main' into duration_time

7200a3a

Simplify dag node indexing

6c56c3e

Expand docs

af6c739

Fix handling for StandardInstruction in rust and add first test

0fe8e0d

Expand test coverage

d46eb5e

mtreinish marked this pull request as ready for review February 17, 2025 21:25

mtreinish requested a review from a team as a code owner February 17, 2025 21:25

mtreinish changed the title ~~[WIP] Add method to compute estimated duration of scheduled circuit~~ Add method to compute estimated duration of scheduled circuit Feb 17, 2025

mtreinish added 2 commits February 18, 2025 16:41

Merge remote-tracking branch 'origin/main' into duration_time

877608f

Fix lint

bb65d6e

ElePT mentioned this pull request Feb 19, 2025

Update op_start_times docstring #13880

Open

ElePT approved these changes Feb 19, 2025

View reviewed changes

ElePT added this pull request to the merge queue Feb 19, 2025

Merged via the queue into Qiskit:main with commit 8c50ce4 Feb 19, 2025
17 checks passed

mergify bot mentioned this pull request Feb 19, 2025

Add method to compute estimated duration of scheduled circuit (backport #13783) #13881

Merged

1 task

mtreinish deleted the duration_time branch February 19, 2025 13:31

mtreinish mentioned this pull request Feb 19, 2025

Fix arithmetic bug in estimate_duration method #13888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add method to compute estimated duration of scheduled circuit #13783

Add method to compute estimated duration of scheduled circuit #13783

mtreinish commented Feb 3, 2025 •

edited

Loading

mtreinish commented Feb 3, 2025

mergify bot commented Feb 3, 2025 •

edited

Loading

coveralls commented Feb 3, 2025 •

edited

Loading

jakelishman left a comment

jakelishman Feb 3, 2025

mtreinish Feb 3, 2025

mtreinish Feb 18, 2025

ElePT Feb 19, 2025 •

edited

Loading

jakelishman Feb 3, 2025

mtreinish Feb 17, 2025

1ucian0 left a comment •

edited

Loading

qiskit-bot commented Feb 17, 2025

mtreinish commented Feb 17, 2025

ElePT left a comment •

edited

Loading

		available through the :attr:`duration`, :attr:`unit` and :attr:`op_start_times` attributes.
		available through the :meth:`estimate_duration` method and :attr:`op_start_times` attribute.

Add method to compute estimated duration of scheduled circuit #13783

Add method to compute estimated duration of scheduled circuit #13783

Conversation

mtreinish commented Feb 3, 2025 • edited Loading

Summary

Details and comments

mtreinish commented Feb 3, 2025

mergify bot commented Feb 3, 2025 • edited Loading

✅ Backports have been created

coveralls commented Feb 3, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13400631790

Details

💛 - Coveralls

jakelishman left a comment

Choose a reason for hiding this comment

jakelishman Feb 3, 2025

Choose a reason for hiding this comment

mtreinish Feb 3, 2025

Choose a reason for hiding this comment

mtreinish Feb 18, 2025

Choose a reason for hiding this comment

ElePT Feb 19, 2025 • edited Loading

Choose a reason for hiding this comment

jakelishman Feb 3, 2025

Choose a reason for hiding this comment

mtreinish Feb 17, 2025

Choose a reason for hiding this comment

1ucian0 left a comment • edited Loading

Choose a reason for hiding this comment

qiskit-bot commented Feb 17, 2025

mtreinish commented Feb 17, 2025

ElePT left a comment • edited Loading

Choose a reason for hiding this comment

mtreinish commented Feb 3, 2025 •

edited

Loading

mergify bot commented Feb 3, 2025 •

edited

Loading

coveralls commented Feb 3, 2025 •

edited

Loading

ElePT Feb 19, 2025 •

edited

Loading

1ucian0 left a comment •

edited

Loading

ElePT left a comment •

edited

Loading