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

ZNE options validation fix #1588

Merged
merged 4 commits into from
Apr 8, 2024
Merged

ZNE options validation fix #1588

merged 4 commits into from
Apr 8, 2024

Conversation

gadial
Copy link
Collaborator

@gadial gadial commented Apr 7, 2024

Summary

This PR fixes some error in the way ZNE options are validated and tested.

Details and comments

Currently, ZNE validation and testing have the following problems:

  1. The _validate_zne_noise_factors method passes in the case the noise factors are the empty list [] although this is an invalid input, since the initial check if self.extrapolator and self.noise_factors considers self.noise_factors to be False in case it is an empty list.
  2. ZNE tests pass for wrong reason. e.g. for {"resilience": {"zne": {"noise_factors": [1]}}} the test_bad_inputs test succeeds because ValidationError is thrown even though the intended error (not enough noise factors) is not tested, since this error is masked by the error of not having zne_mitigation: True in the resilience dictionary.

These problems are handled in the following manner:

  1. _validate_zne_noise_factors now never skips. If noise factors or extrapolators are missing, it uses the default values described in the class's file.
  2. test_bad_inputs now uses assertRaisesRegex with relevant error message for each test case, to avoid passing when irrelevant exceptions are thrown.
  3. Another tests were added to test_bad_inputs for the case of empty noise factors list and for the case of missing zne_mitigation: True.

@gadial gadial requested a review from kt474 April 7, 2024 14:12
@coveralls
Copy link

coveralls commented Apr 7, 2024

Pull Request Test Coverage Report for Build 8603463800

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 15 of 15 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 83.488%

Totals Coverage Status
Change from base Build 8578891638: 0.04%
Covered Lines: 6224
Relevant Lines: 7455

💛 - Coveralls

@kt474 kt474 added the Changelog: Bugfix Include in the Fixed section of the changelog label Apr 8, 2024
@kt474 kt474 merged commit 64f6e66 into Qiskit:main Apr 8, 2024
18 checks passed
@gadial gadial deleted the zne_validation_fix branch April 8, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the Fixed section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants