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

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

Merged
merged 5 commits into from
Feb 20, 2025

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Feb 19, 2025

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 is an automatic backport of pull request #13783 done by [Mergify](https://mergify.com).

* 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)
@mergify mergify bot requested a review from a team as a code owner February 19, 2025 10:12
@qiskit-bot
Copy link
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

@github-actions github-actions bot 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 19, 2025
@github-actions github-actions bot added this to the 1.4.0 milestone Feb 19, 2025
@coveralls
Copy link

coveralls commented Feb 19, 2025

Pull Request Test Coverage Report for Build 13433621755

Details

  • 73 of 91 (80.22%) changed or added relevant lines in 5 files are covered.
  • 19 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.03%) to 88.864%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/accelerate/src/circuit_duration.rs 50 68 73.53%
Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/two_qubit_decompose.rs 1 91.96%
crates/qasm2/src/expr.rs 1 94.02%
crates/qasm2/src/lex.rs 5 92.48%
crates/qasm2/src/parse.rs 12 97.15%
Totals Coverage Status
Change from base Build 13372661553: -0.03%
Covered Lines: 79159
Relevant Lines: 89079

💛 - Coveralls

ElePT
ElePT previously approved these changes Feb 20, 2025
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM! I did notice that the fix in main included an additional test that I think would be good to have here too, but in any case what really counts is the fix and that is in, so I wouldn't block on that. I copy pasted the test in a suggestion, I think that should not break lint but who knows.

ElePT
ElePT previously approved these changes Feb 20, 2025
@ElePT ElePT enabled auto-merge February 20, 2025 10:00
@ElePT ElePT added this pull request to the merge queue Feb 20, 2025
Merged via the queue into stable/1.4 with commit 744037a Feb 20, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants