Skip to content

Draft: Improve performance of QuantumCircuit.measure_all and QuantumCircuit.barrier by using _append fast path#11008

Closed
eendebakpt wants to merge 2 commits into
Qiskit:mainfrom
eendebakpt:fast_barrier_measure
Closed

Draft: Improve performance of QuantumCircuit.measure_all and QuantumCircuit.barrier by using _append fast path#11008
eendebakpt wants to merge 2 commits into
Qiskit:mainfrom
eendebakpt:fast_barrier_measure

Conversation

@eendebakpt
Copy link
Copy Markdown
Contributor

Summary

Improve performance of QuantumCircuit.measure_all and QuantumCircuit.barrier by using _append fast path

Details and comments

The register argument are obtained from the QuantumCircuit directly, so do not need to be validated.

@eendebakpt eendebakpt requested a review from a team as a code owner October 12, 2023 19:33
@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Oct 12, 2023
@qiskit-bot
Copy link
Copy Markdown
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

Copy link
Copy Markdown
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

When dropping to the lower-level _append primitive, there a quite a lot of additional considerations that must be validated by the caller:

  • any control-flow builder scopes must be resolved
  • the qubit counts and clbit counts should be known to match the instruction (Measure is a 1q/1c operation, for example)
  • _append should take a single CircuitInstruction (which comes with its own validation requirements), not three arguments

It's worth looking at end of append's implementation to be sure that all the invariants it enforces are being respected here as well.

Out of curiosity, in what situations is this function even visible in performance benchmarking?

@eendebakpt
Copy link
Copy Markdown
Contributor Author

When dropping to the lower-level _append primitive, there a quite a lot of additional considerations that must be validated by the caller:

  • any control-flow builder scopes must be resolved
  • the qubit counts and clbit counts should be known to match the instruction (Measure is a 1q/1c operation, for example)
  • _append should take a single CircuitInstruction (which comes with its own validation requirements), not three arguments

It's worth looking at end of append's implementation to be sure that all the invariants it enforces are being respected here as well.

Out of curiosity, in what situations is this function even visible in performance benchmarking?

Thanks for the info. The barrier and (to lesser extend measure) show up in randomized benchmarking. I did some more tests, but the performance gain is not so high as the profiling suggests. I will close the PR.

@eendebakpt eendebakpt closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PR PRs from contributors that are not 'members' of the Qiskit repo

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants