Skip to content

Add caching to value_equality_values decorator for auto generated methods.#6275

Merged
tanujkhattar merged 4 commits intoquantumlib:masterfrom
tanujkhattar:cache_value_equality
Sep 6, 2023
Merged

Add caching to value_equality_values decorator for auto generated methods.#6275
tanujkhattar merged 4 commits intoquantumlib:masterfrom
tanujkhattar:cache_value_equality

Conversation

@tanujkhattar
Copy link
Copy Markdown
Collaborator

@tanujkhattar tanujkhattar commented Aug 30, 2023

Fixes #6272
cc #6097

Benchmarks can be seen by running

>>> gate = cirq.PhasedXZGate.from_matrix(cirq.testing.random_unitary(2))
>>> %timeit hash(gate)

Before

2.67 µs ± 52.8 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

After

159 ns ± 2.75 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

A more real world example would be QROM in Cirq-FT, where value_equality_values returns all the data to be loaded by the QROM. Benchmarks as follows

>>> qrom = cirq_ft.QROM.build([1, 2, 3, 4, 5] * 10_000)
>>> %timeit cirq_ft.t_complexity(qrom)

Before

# 3.39 ms ± 32.2 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

After

# First Run
# The slowest run took 14.37 times longer than the fastest. This could mean that an intermediate result is being cached.
# 6.05 µs ± 9.3 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

# Second Run
# 1.85 µs ± 37.4 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

@tanujkhattar tanujkhattar requested review from a team, cduck and vtomole as code owners August 30, 2023 18:35
@tanujkhattar tanujkhattar requested a review from viathor August 30, 2023 18:35
@tanujkhattar tanujkhattar changed the title Add caching to value_equality_values decorator for auto generated methods. Add caching to value_equality_values decorator for auto generated methods. Aug 30, 2023
@CirqBot CirqBot added the size: S 10< lines changed <50 label Aug 30, 2023
Comment thread cirq-core/cirq/value/value_equality_attr.py Outdated
Comment thread cirq-core/cirq/value/value_equality_attr.py Outdated
Copy link
Copy Markdown
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

Nit: _compat.cached_method(values_getter) can be reused so that the values_getter is evaluated once. Otherwise LGTM.

@tanujkhattar tanujkhattar added the BREAKING CHANGE For pull requests that are important to mention in release notes. label Aug 30, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 31, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (0e80fa5) 97.87% compared to head (3326064) 97.88%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6275   +/-   ##
=======================================
  Coverage   97.87%   97.88%           
=======================================
  Files        1106     1106           
  Lines       95710    95717    +7     
=======================================
+ Hits        93679    93690   +11     
+ Misses       2031     2027    -4     
Files Changed Coverage Δ
cirq-core/cirq/ops/dense_pauli_string.py 97.97% <100.00%> (+0.01%) ⬆️
cirq-core/cirq/ops/linear_combinations.py 99.79% <100.00%> (ø)
cirq-core/cirq/value/value_equality_attr.py 96.87% <100.00%> (+0.26%) ⬆️

... and 1 file with indirect coverage changes

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

@tanujkhattar tanujkhattar merged commit eddb2d9 into quantumlib:master Sep 6, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
…ethods. (quantumlib#6275)

* Add caching to value_equality_values decorator for auto generated methods.

* Fix pylint and formatting errors

* Address nits, fix bugs and make PauliSum unhashable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/performance BREAKING CHANGE For pull requests that are important to mention in release notes. size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Caching value_equality_values?

4 participants