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

Cache mapped wires in Device #1270

Merged
merged 19 commits into from
May 6, 2021
Merged

Cache mapped wires in Device #1270

merged 19 commits into from
May 6, 2021

Conversation

antalszava
Copy link
Contributor

Context:

Wires used internally in devices get mapped based on the initial map wiring defined. Although doing this wire mapping is not too computationally intensive when done a single time, when running an optimization task where it's done thousands of times, the cost for computing mapped wires is non-negligible in terms of time.

Often devices query the mapping for the same wires several times. As the wire mapping of a device is set only when the device is created, it is safe to assume that it's immutable. Therefore we can assume that the value of mapping wires will also not change over the life of the device.

Description of the Change:

  • When calling map_wires, the Device class now caches the key-value pair of the wires and the mapped wires in a dictionary
  • The hash of a Wires object is cached

Benefits:
More performant device and Wires class usage.

Possible Drawbacks:
N/A

Related GitHub Issues:
N/A

@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2021

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented May 3, 2021

Codecov Report

Merging #1270 (3fb88d3) into master (9f01932) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1270   +/-   ##
=======================================
  Coverage   98.13%   98.13%           
=======================================
  Files         147      147           
  Lines       11250    11256    +6     
=======================================
+ Hits        11040    11046    +6     
  Misses        210      210           
Impacted Files Coverage Δ
pennylane/_device.py 96.72% <100.00%> (+0.02%) ⬆️
pennylane/devices/default_qubit.py 100.00% <100.00%> (ø)
pennylane/wires.py 98.71% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f01932...3fb88d3. Read the comment docs.

@antalszava
Copy link
Contributor Author

Some benchmarks:

GradientComputation_light.time_gradient

Commit 8fe31ad

          --                                        n_layers / interface      
          --------- -----------------------------------------------------------------------------
           n_wires   3 / autograd    3 / tf    3 / torch    6 / autograd    6 / tf    6 / torch  
          ========= ============== ========== ============ ============== ========== ============
              2      9.21±0.09ms    33.6±6ms   14.7±0.2ms     15.0±2ms     52.0±2ms   40.5±0.9ms 
              5       20.0±0.1ms    92.0±3ms   78.2±0.9ms     36.5±1ms     172±8ms     328±40ms  
          ========= ============== ========== ============ ============== ========== ============

Commit 051f845

          --                                        n_layers / interface                               
          --------- -----------------------------------------------------------------------------------
           n_wires   3 / autograd      3 / tf      3 / torch    6 / autograd     6 / tf     6 / torch  
          ========= ============== ============= ============= ============== ============ ============
              2      5.13±0.03ms    16.6±0.08ms   8.26±0.04ms    7.91±0.1ms    29.8±0.1ms   22.6±0.2ms 
              5       11.0±0.2ms      45.2±4ms      43.6±7ms      22.6±2ms      85.3±1ms     154±20ms  
          ========= ============== ============= ============= ============== ============ ============

ML_light

Commit 8fe31ad

Mem:

            autograd   256M 
             torch     283M 
               tf      303M 

Time:

            autograd   10.9±0.4s 
             torch       failed  
               tf      23.8±0.4s 

Commit 051f845

Mem:

            autograd   256M 
             torch     284M 
               tf      303M 

Time:

            autograd   9.55±0.3s 
             torch       failed  
               tf      23.2±0.3s 

tests/test_wires.py Outdated Show resolved Hide resolved
@@ -340,6 +341,9 @@ def map_wires(self, wires):
Returns:
Wires: wires with new labels
"""
if wires in self._cached_wires:
return self._cached_wires[wires]

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice solution for now. When we are refactoring the device class (if this ever happens :) I will help turning all the current patches into a runtime-conscious solution overall!

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Looks good from my end! Agree with @mariaschuld re: having a more runtime specific solution.

A very minor thought that crossed my head while writing this - would @functools.lru_cache achieve the same result with less complexity?

pennylane/_device.py Outdated Show resolved Hide resolved
@josh146 josh146 merged commit 204516f into master May 6, 2021
@josh146 josh146 deleted the cache_mapped_wires branch May 6, 2021 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants