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

Cleaned code involving noise_amplifier options #1184

Closed
wants to merge 1 commit into from

Conversation

merav-aharoni
Copy link
Contributor

Summary

Removed the code involving noise_amplifier documentation and checking, since currently only one option is supported.

Details and comments

See comment in #1093.

…, since currently only one option is supported
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6704017400

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

Totals Coverage Status
Change from base Build 6670772616: 0.008%
Covered Lines: 2524
Relevant Lines: 3351

💛 - Coveralls

@merav-aharoni merav-aharoni marked this pull request as ready for review October 31, 2023 12:54
@merav-aharoni merav-aharoni requested a review from kt474 October 31, 2023 12:54
@@ -53,7 +49,7 @@ class ResilienceOptions:
Default: ``None``, and ``LinearExtrapolator`` if resilience level is 2.
"""

noise_amplifier: NoiseAmplifierType = None
noise_amplifier: NoiseAmplifierType = "LocalFoldingAmplifier"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't merge this until there is an update on the server side right? Because this would add noise_amplifier to every job, including those without resilience_level 2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, this will have no effect on other resilience levels, because ZNE won't be run. @jyu00 - any comment on this?

@drew-distefano drew-distefano requested a review from jyu00 December 1, 2023 18:11
@merav-aharoni merav-aharoni deleted the noise_amplifier branch December 11, 2023 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants