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

Speed up cirq.map_operations and cirq.map_operations_and_unroll #6250

Merged
merged 8 commits into from
Aug 24, 2023

Conversation

tanujkhattar
Copy link
Collaborator

@tanujkhattar tanujkhattar commented Aug 16, 2023

Fixes #6173
Part of #6097

This leads to a roughly ~4-5x improvement in performance for cirq.map_operations and cirq.map_operations_and_unroll. See #6173 (comment) for a detailed use-case specific analysis.

The primary reason behind the speedup is to get rid of repeated instantiations of cirq.Circuit, which was being done for once for every call to map_func(op, i). The intermediate circuits were constructed mainly for circuit alignment. This PR instead uses the circuit construction strategy similar to the one used in

def _load_contents_with_earliest_strategy(self, contents: 'cirq.OP_TREE'):
which ensures that we take care of the correct alignment with minimal overhead and call the cirq.Circuit constructor only once at the end.

@tanujkhattar tanujkhattar requested review from vtomole, cduck and a team as code owners August 16, 2023 21:21
@tanujkhattar tanujkhattar changed the title Speed up cirq.map_operations and cirq.map_operations_and_unroll Speed up cirq.map_operations and cirq.map_operations_and_unroll Aug 16, 2023
@CirqBot CirqBot added the size: M 50< lines changed <250 label Aug 16, 2023
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (6abc740) 97.60% compared to head (0f159cf) 97.60%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6250   +/-   ##
=======================================
  Coverage   97.60%   97.60%           
=======================================
  Files        1116     1116           
  Lines       95768    95848   +80     
=======================================
+ Hits        93476    93556   +80     
  Misses       2292     2292           
Files Changed Coverage Δ
...q-core/cirq/transformers/transformer_primitives.py 99.57% <100.00%> (+0.06%) ⬆️
cirq-ft/cirq_ft/algos/qrom.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/algos/qrom_test.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/algos/unary_iteration_gate.py 96.26% <100.00%> (+0.14%) ⬆️
cirq-ft/cirq_ft/infra/qubit_manager.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/infra/qubit_manager_test.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NoureldinYosri
Copy link
Collaborator

could you explain the change in the description? also what is happening with the mypy script?

@tanujkhattar
Copy link
Collaborator Author

@NoureldinYosri Added a detailed description and the change to mypy script was a mistake so I've reverted it.

Copy link
Collaborator

@NoureldinYosri NoureldinYosri 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 a few nits and documentation

@tanujkhattar tanujkhattar enabled auto-merge (squash) August 24, 2023 17:38
@tanujkhattar tanujkhattar merged commit 7ed95aa into quantumlib:master Aug 24, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
…uantumlib#6250)

* Speed up cirq.map_operations and cirq.map_operations_and_unroll

* Mypy typing and minor bug fixes

* Fix pylint

* Revert unrelated change to mypy script

* Address nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hardware inverse transformer faster
4 participants