Update measurements tests for DQ2; create legacy tests folder #4574
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
|
Hello. You may have forgotten to update the changelog!
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4574 +/- ##
==========================================
- Coverage 99.65% 99.65% -0.01%
==========================================
Files 376 376
Lines 33148 33156 +8
==========================================
+ Hits 33035 33042 +7
- Misses 113 114 +1
☔ View full report in Codecov by Sentry. |
mudit2812
approved these changes
Sep 6, 2023
mudit2812
pushed a commit
that referenced
this pull request
Sep 8, 2023
**Context:** Measurement tests are focused on legacy devices, but we should use the new device API for these tests. **Description of the Change:** Here's what I did, chronologically: 1. copy-paste every measurement test file into the `legacy` sub-folder 2. change the original files to use `default.qubit` (not DQL), change `default.qubit` to point to DQ2, fix tests so they work with DQ2 (they all passed) 3. Put back `default.qubit` to point to DQL, re-run non-legacy tests to see which ones fail, mark those as xfail (for now) 4. Remove all tests in the `legacy` folder that double-cover anything (in other words, any tests that didn't use DQL) Step 2 above involved some changes to PennyLane source code, and they are: 1. Introduce `MutualInfoMP.map_wires` 2. Change the qinfo transforms to use tape wires if device wires aren't present (I will probably revert this once device wires are supported for DQ2) 3. Fix `sampling.py` to not squeeze any CountsMP result (batched results also shouldn't be squeezed, but they are lists of dicts instead of just dicts) 4. EDIT: Update `ProbabilityMP.marginal_prob` to just return `prob` if `self.wires` is None. This was commented out before, and I'm not sure why. **Benefits:** - Migrating to DQ2 will be much easier, and we'll have better test coverage for measurements with the new device API - `qml.probs()` did not work with DQ2 before this little change D: **Possible Drawbacks:** Slightly more tests. Think of this as effectively adding one more device to test with whenever doing device integration tests. **Follow-Up:** I made [this story](https://app.shortcut.com/xanaduai/story/44915/update-setup-py-to-point-default-qubit-to-defaultqubit2) and mentioned the removal of xfails in it.
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:
Measurement tests are focused on legacy devices, but we should use the new device API for these tests.
Description of the Change:
Here's what I did, chronologically:
legacysub-folderdefault.qubit(not DQL), changedefault.qubitto point to DQ2, fix tests so they work with DQ2 (they all passed)default.qubitto point to DQL, re-run non-legacy tests to see which ones fail, mark those as xfail (for now)legacyfolder that double-cover anything (in other words, any tests that didn't use DQL)Step 2 above involved some changes to PennyLane source code, and they are:
MutualInfoMP.map_wiressampling.pyto not squeeze any CountsMP result (batched results also shouldn't be squeezed, but they are lists of dicts instead of just dicts)ProbabilityMP.marginal_probto just returnprobifself.wiresis None. This was commented out before, and I'm not sure why.Benefits:
qml.probs()did not work with DQ2 before this little change D:Possible Drawbacks:
Slightly more tests. Think of this as effectively adding one more device to test with whenever doing device integration tests.
Follow-Up:
I made this story and mentioned the removal of xfails in it.