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

Fixing BlockCollapser with Clbits #9823

Merged
merged 7 commits into from
May 26, 2023

Conversation

alexanderivrii
Copy link
Contributor

Summary

Fixes #9814.

I have added @chriseclectic as a co-author: the code for fixing BlockCollapser and DAGCircuit, as well as testing is mostly his.

Details and comments

Classical bits are now treated correctly in BlockCollapser and in replace_block_with_op.

@alexanderivrii alexanderivrii requested a review from a team as a code owner March 20, 2023 12:48
@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

@coveralls
Copy link

coveralls commented Mar 20, 2023

Pull Request Test Coverage Report for Build 4843816267

  • 32 of 32 (100.0%) changed or added relevant lines in 3 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.01%) to 85.947%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 91.65%
Totals Coverage Status
Change from base Build 4829590605: 0.01%
Covered Lines: 71218
Relevant Lines: 82863

💛 - Coveralls

Copy link
Member

@chriseclectic chriseclectic left a comment

Choose a reason for hiding this comment

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

Added comments to get this to this work with full ClassicalRegister conditionals as well as Clbit conditionals

qiskit/dagcircuit/collect_blocks.py Outdated Show resolved Hide resolved
qiskit/dagcircuit/collect_blocks.py Outdated Show resolved Hide resolved
qiskit/dagcircuit/collect_blocks.py Outdated Show resolved Hide resolved
Copy link
Member

@chriseclectic chriseclectic left a comment

Choose a reason for hiding this comment

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

LGTM

@alexanderivrii alexanderivrii added this to the 0.25.0 milestone May 16, 2023
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.

This LGTM, thanks for fixing this.

@mtreinish mtreinish added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels May 26, 2023
@mtreinish mtreinish modified the milestones: 0.25.0, 0.24.1 May 26, 2023
@mtreinish mtreinish added this pull request to the merge queue May 26, 2023
Merged via the queue into Qiskit:main with commit 788b89d May 26, 2023
mergify bot pushed a commit that referenced this pull request May 26, 2023
* Bug fix: collapsing blocks with clbits

Co-authored-by: chriseclectic <[email protected]>

* release notes

* similar fix to DAGDependency

* additional fixes to handle classical bits in conditions

* fixing lint issue

* Fixing handling of conditions over full registers, following review comments

---------

Co-authored-by: chriseclectic <[email protected]>
(cherry picked from commit 788b89d)
mtreinish pushed a commit that referenced this pull request May 26, 2023
* Bug fix: collapsing blocks with clbits

Co-authored-by: chriseclectic <[email protected]>

* release notes

* similar fix to DAGDependency

* additional fixes to handle classical bits in conditions

* fixing lint issue

* Fixing handling of conditions over full registers, following review comments

---------

Co-authored-by: chriseclectic <[email protected]>
(cherry picked from commit 788b89d)

Co-authored-by: Alexander Ivrii <[email protected]>
@alexanderivrii alexanderivrii deleted the collect-clbits-fix branch June 11, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BlockCollapser doesnt work with Clbits
5 participants