Changes to readout mitigator tests #7482
Conversation
Pull Request Test Coverage Report for Build 1858802700
💛 - Coveralls |
gadial
changed the title
| @property | ||
| def settings(self) -> Dict: | ||
| """Return settings.""" | ||
| return {"assignment_matrices": self._assignment_mats, "qubits": self._qubits} | ||
|
|
There was a problem hiding this comment.
Why not adding it to the super class BaseReadoutMitigator?
There was a problem hiding this comment.
This is since the assignment matrix is a 2^n-by-2^n matrix (where n is the number of qubits), so it will not be possible to generate it for advanced mitigation methods (such as M3).
| ) | ||
| mitigated_error = self.compare_results(counts_ideal, mitigated_probs) | ||
| self.assertTrue( | ||
| mitigated_error < unmitigated_error * 0.8, |
There was a problem hiding this comment.
Where does the factor of 0.8 come from? Is this absolutely 100% guaranteed to always succeed - is there any fuzziness to it at all?
There was a problem hiding this comment.
I know of no general result here. This specific number was chosen because it works with the current data, allowing for detections of changes to the mitigators which make it fail on this specific data. This was done with the hard-coded numbers in mind; I agree that by relying on getting the numbers from the fake backend we introduce fuzziness.
| expected_assignment_matrix = np.kron( | ||
| np.kron(assignment_matrices[2], assignment_matrices[1]), assignment_matrices[0] | ||
| ) | ||
| expected_mitigation_matrix = np.linalg.inv(expected_assignment_matrix) |
There was a problem hiding this comment.
This is 100x better thanks - I can super clearly see what's going on, and it's obvious to me that this should be a valid mitigation strategy.
jakelishman
added
automerge
Changelog: Bugfix
* Bugfix in local mitigator and a relevant test * Name fix * Refactoring mitigation tests * Cleanup * Linting * Linting * Small bugfix and a test for it * Fix to rng usage * Linting * Changing test sensitivity * Added settings property to mitigator to allow JSON encoding when given as experiment result * Linting * Linting * Linting * Release note relating to the bugfix * Get the assignment matrices implicitly * Using more specific assertions * Removing magic constants * Make initialization conform to settings * Linting * Removing testcase which might fail on some machines * Removing testcase which might fail on some machines * Linting * Reword release note * Reword release note Co-authored-by: Jake Lishman <jake.lishman@ibm.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit ff267b5) # Conflicts: # test/python/result/test_mitigators.py
jakelishman
removed
the
stable backport potential
Summary
Details and comments
This PR addresses the use of constants in the main class for testing the readout mitigators. This can be replaced by one the fly computation of the expected results.
A
settingsmethod is added to the mitigators to enable JSON encoding/decoding.A bug in
LocalReadoutMitigator.assignment_matrixis fixed.