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.

Sign up for GitHub

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

Jump to bottom

Speed up circuit construction when contents are a list of moments #5898

Merged
merged 2 commits into from
Sep 29, 2022
Merged

Speed up circuit construction when contents are a list of moments #5898

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef0e9be
Speed up circuit construction when contents are a list of moments
tanujkhattar Sep 28, 2022
4b223a0
Add explicit cast to fix mypy errors
tanujkhattar Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions cirq-core/cirq/circuits/circuit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1741,11 +1741,15 @@ def __init__(
circuit.
"""
self._moments: List['cirq.Moment'] = []
flattened_contents = tuple(ops.flatten_to_ops_or_moments(contents))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably go for list instead, since we don't need immutability here, but it'd be interesting to know if there is any performance difference one way or the other. If not, then up to you whether to use list or tuple.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tuples are usually slightly faster than lists because they don't overallocate like lists (to make append faster) so the memory footprint is more compact. Also, tuples don't need to go through an additional level of indirection when accessing elements like lists; see https://stackoverflow.com/a/22140115 for more details

As an example, here are numbers for running the operation by operation construction benchmark (in which the flattened_contents would contain all the operations and hence have a large size).

Using tuples for flattened_contents:

[  0.00%] ·· Benchmarking existing-py_usr_local_google_home_tanujkhattar_anaconda3_envs_cirq_bin_python
[ 50.00%] ··· Running (circuit_construction.SurfaceCodeRotatedMemoryZ.time_circuit_construction_operations_by_operation--).    
[100.00%] ··· circuit_construction.SurfaceCodeRotatedMemoryZ.time_circuit_construction_operations_by_operation                                                                                                                                                                          ok
[100.00%] ··· ========== ============
               distance              
              ---------- ------------
                  3       2.68±0.1ms 
                  5       18.2±0.7ms 
                  7        72.2±2ms  
                  9        198±6ms   
                  11       464±10ms  
                  13       933±40ms  
                  15      1.68±0.06s 
                  17      2.95±0.09s 
                  19      4.73±0.2s  
                  21      6.39±0.02s 
                  23      9.50±0.03s 
                  25      13.7±0.4s  
              ========== ============

Using lists for flattened_contents:

[  0.00%] ·· Benchmarking existing-py_usr_local_google_home_tanujkhattar_anaconda3_envs_cirq_bin_python
[ 50.00%] ··· Running (circuit_construction.SurfaceCodeRotatedMemoryZ.time_circuit_construction_operations_by_operation--).
[100.00%] ··· circuit_construction.SurfaceCodeRotatedMemoryZ.time_circuit_construction_operations_by_operation                                                                                                                                                                          ok
[100.00%] ··· ========== ============
               distance              
              ---------- ------------
                  3       2.81±0.2ms 
                  5       20.3±0.8ms 
                  7        79.9±3ms  
                  9        204±4ms   
                  11       467±10ms  
                  13       934±10ms  
                  15      1.75±0.1s  
                  17      2.92±0.08s 
                  19      4.42±0.1s  
                  21      6.65±0.02s 
                  23      9.85±0.04s 
                  25      13.8±0.2s  
              ========== ============

if all(isinstance(c, Moment) for c in flattened_contents):
self._moments[:] = cast(Iterable[Moment], flattened_contents)
return
with _compat.block_overlapping_deprecation('.*'):
if strategy == InsertStrategy.EARLIEST:
self._load_contents_with_earliest_strategy(contents)
self._load_contents_with_earliest_strategy(flattened_contents)
else:
self.append(contents, strategy=strategy)
self.append(flattened_contents, strategy=strategy)

@classmethod
def _from_moments(cls, moments: Iterable['cirq.Moment']) -> 'Circuit':
Expand Down