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

Cleanup timeline drawer with schedule analysis pass. #7935

Merged
merged 11 commits into from
Jun 16, 2022

Conversation

nkanazawa1989
Copy link
Contributor

Summary

Timeline drawer implements own scheduler but this is no longer necessary since we have implemented absolute-time scheduling pass in #7709.

This PR copies this scheduling outcome to the output circuit and let the timeline drawer use this information. Because this schedule contains t0 for classical registers, now timeline drawer can properly show time slots for classical registers.

Details and comments

from qiskit import QuantumCircuit, transpile
from qiskit.test.mock import FakeMontreal
from qiskit.visualization.timeline import draw as draw_timeline

backend = FakeMontreal()

qct = transpile(
    qc, backend, initial_layout=[0, 1], coupling_map=[[0, 1], [1, 0]], scheduling_method="alap"
)
draw_timeline(qct, show_clbits=True)

image

@nkanazawa1989 nkanazawa1989 requested review from a team and nonhermitian as code owners April 13, 2022 18:34
@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 the following people are requested to review this:

  • @Qiskit/terra-core

@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Apr 13, 2022
@mtreinish mtreinish added this to the 0.21 milestone Apr 13, 2022
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, just one small inline comment. Also I think maybe having a test that we raise an Attribute error on an unscheduled circuit would be good just so we have the coverage.

qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
nkanazawa1989 and others added 3 commits April 14, 2022 04:51
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@coveralls
Copy link

coveralls commented Apr 13, 2022

Pull Request Test Coverage Report for Build 2510201605

  • 45 of 55 (81.82%) changed or added relevant lines in 4 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.01%) to 84.267%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/basepasses.py 1 6 16.67%
qiskit/visualization/timeline/core.py 32 37 86.49%
Files with Coverage Reduction New Missed Lines %
qiskit/circuit/quantumcircuit.py 3 94.12%
Totals Coverage Status
Change from base Build 2505540605: -0.01%
Covered Lines: 54816
Relevant Lines: 65050

💛 - Coveralls

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Overall this LGTM, just one question about the release note

qc, backend, initial_layout=[0, 1], coupling_map=[[0, 1]], scheduling_method="alap"
)
scheduled_insts = list(zip(qct.op_start_times, qct.data))

Copy link
Member

Choose a reason for hiding this comment

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

Did you want to add a deprecation note about visualizing a timeline for an unscheduled/untranspiled circuit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Matthew I've forgotten to write. Added in 8891220

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the update

@mtreinish mtreinish added Changelog: Deprecation Include in "Deprecated" section of changelog automerge labels Jun 16, 2022
@mergify mergify bot merged commit 7391168 into Qiskit:main Jun 16, 2022
@nkanazawa1989 nkanazawa1989 deleted the upgrade/cleanup_timeline_draw branch November 25, 2022 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Deprecation Include in "Deprecated" section of changelog Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants