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

Alternative solution to representation caching #431

Closed
wants to merge 11 commits into from
Closed

Conversation

apchytr
Copy link
Collaborator

@apchytr apchytr commented Jul 9, 2024

Opening for discussion

Context: This PR is meant as a proposed alternative to representation caching. By using 'lazy evaluation' in representation/ansatz we can avoid having potentially costly computations just to initialize the representation for calls such as obj.representation.__class__.

Description of the Change: Ansatz no longer calls backend methods in the init. Instead, the original parameters are stored (mat, vec, array for PolyExpAnsatz or array for ArrayAnsatz) and backend methods such as astensor and at_least3d are only called when the property itself is called (then subsequently stored to avoid repeat calls). In addition to this the representation/ansatz can now also accept a function and some kwargs in substitution of specifying an (A, b, c) through a from_generator class method.

Benefits: Based on some initial profiling initializing a representation/ansatz is more efficient.

mrmustard/physics/ansatze.py Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 97.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.87%. Comparing base (31a1f8a) to head (ad8848d).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #431      +/-   ##
===========================================
+ Coverage    87.80%   87.87%   +0.06%     
===========================================
  Files           81       81              
  Lines         6230     6282      +52     
===========================================
+ Hits          5470     5520      +50     
- Misses         760      762       +2     
Files Coverage Δ
mrmustard/lab_dev/circuit_components_utils.py 100.00% <100.00%> (ø)
mrmustard/lab_dev/states/base.py 96.91% <100.00%> (ø)
mrmustard/lab_dev/states/states.py 100.00% <100.00%> (ø)
...mustard/lab_dev/transformations/transformations.py 100.00% <100.00%> (ø)
mrmustard/physics/representations.py 97.64% <100.00%> (ø)
mrmustard/physics/triples.py 100.00% <100.00%> (ø)
mrmustard/physics/ansatze.py 95.31% <97.14%> (+0.23%) ⬆️

Continue to review full report in Codecov by Sentry.

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

Copy link
Collaborator

@ziofil ziofil left a comment

Choose a reason for hiding this comment

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

Beautiful concept, I really like it! Before we lock it in could we keep A,b,c in the init of Bargmann, and have a separate class method that takes fn, **kwargs instead? (it's a bit odd to have A,b,c and (e.g.) cov, means as arguments of the init)

@apchytr apchytr closed this Jul 15, 2024
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

2 participants