Fix a sorting order in parameter-shift terms #5583
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5583 +/- ##
==========================================
- Coverage 99.68% 99.68% -0.01%
==========================================
Files 412 412
Lines 38645 38354 -291
==========================================
- Hits 38525 38232 -293
- Misses 120 122 +2 ☔ View full report in Codecov by Sentry. |
mudit2812
approved these changes
May 10, 2024
trbromley
reviewed
May 13, 2024
| @@ -6,6 +6,9 @@ | |||
|
|
|||
| <h3>Improvements 🛠</h3> | |||
|
|
|||
| * The sorting order of parameter-shift terms is now guaranteed to resolve ties in the absolute value with the sign of the shifts. | |||
| [(#5582)](https://github.com/PennyLaneAI/pennylane/pull/5582) | |||
There was a problem hiding this comment.
Was this the wrong PR number?
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:
Currently,
process_shiftssorts shifts according to the absolute value of the shift.It uses
kind="stable", which guarantees to keep inputs in their input order if they have same absolute value.While this is stable against repeated calls, the stable order is somewhat arbitrarily depending on how a user/PennyLane
arrived at the input shifts.
This is visible in
tests/gradients/core/test_general_shift_rules.py, where the expected output values sometimes have positive before negative shifts with same magnitude, and sometimes the other way around.Description of the Change:
This PR uses
lexsort, which is likeargsortbut allows us to use the sign of the shift as a secondary sorting key, producing not only stable but also well-defined sorting results.Benefits:
Predictable sorting results beyond the absolute value alone.
Possible Drawbacks:
Related GitHub Issues:
[sc-45043]