Register ParameterizedEvolution #4598
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # tests/devices/test_default_qubit_legacy.py
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4598 +/- ##
==========================================
- Coverage 99.64% 99.64% -0.01%
==========================================
Files 375 375
Lines 33331 33370 +39
==========================================
+ Hits 33214 33252 +38
- Misses 117 118 +1
☔ View full report in Codecov by Sentry. |
|
Hello. You may have forgotten to update the changelog!
|
|
[sc-44329] |
timmysilv
reviewed
Sep 15, 2023
There was a problem hiding this comment.
looks great! just left some comments about batching and import errors, but the rest looks nice 🕴️ also - don't forget a changelog entry!
to fix coverage issues, can you add an integration test that uses the new DQ and custom wires (eg. copy-paste one with regular wires and change them to custom ones like ['a', 'b', 'c']). if you wanna add an explicit test for ParametrizedEvolution.map_wires and double-check the ParametrizedHamiltonians on it also get mapped, it wouldn't hurt :p
lillian542
commented
Sep 18, 2023
mudit2812
reviewed
Sep 18, 2023
lillian542
commented
Sep 18, 2023
timmysilv
approved these changes
Sep 19, 2023
)
mudit2812
approved these changes
Sep 19, 2023
# Conflicts: # doc/releases/changelog-dev.md
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
mudit2812
added a commit
that referenced
this pull request
Sep 19, 2023
**Context:** On the old device API, a `ParametrizedEvolution` uses the default `apply_operation` behaviour if it is preferable to evolve the matrix and apply (small matrix on a large state), but has special handling for if the matrix gets bigger (specifically, if the wires for the matrix exceed half the wires for the state, it evolves the state instead). Adding this made things much faster and people were very happy. **Description of the Change:** Adds the same behaviour here, now based on the size of the state instead of the number of device wires. **Benefits:** Presumably confers the speedups observed on DQL to DQ2. **Possible Drawbacks:** I tested whether the correct function was called based on the number of calls to `math.einsum`, because `mocker.spy` can't access the `devices.qubit.apply_operation` module due to ambiguity between the module and the function of the same name. It's the best thing I could come up with. It does not seem very reliable if anything changes in the future. --------- Co-authored-by: Matthew Silverman <matthews@xanadu.ai> Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context:
On the old device API, a
ParametrizedEvolutionuses the defaultapply_operationbehaviour if it is preferable to evolve the matrix and apply (small matrix on a large state), but has special handling for if the matrix gets bigger (specifically, if the wires for the matrix exceed half the wires for the state, it evolves the state instead). Adding this made things much faster and people were very happy.Description of the Change:
Adds the same behaviour here, now based on the size of the state instead of the number of device wires.
Benefits:
Presumably confers the speedups observed on DQL to DQ2.
Possible Drawbacks:
I tested whether the correct function was called based on the number of calls to
math.einsum, becausemocker.spycan't access thedevices.qubit.apply_operationmodule due to ambiguity between the module and the function of the same name. It's the best thing I could come up with. It does not seem very reliable if anything changes in the future.