Skip to content

Changes that might help with intermittent Stochastic Noise failure - #2346

Merged
maxwhitemet merged 12 commits into
metoppv:masterfrom
gavinevans:mobt_1114_stochastic_noise_failure
Apr 27, 2026
Merged

Changes that might help with intermittent Stochastic Noise failure#2346
maxwhitemet merged 12 commits into
metoppv:masterfrom
gavinevans:mobt_1114_stochastic_noise_failure

Conversation

@gavinevans

Copy link
Copy Markdown
Contributor

Addresses https://github.com/metoppv/mo-blue-team/issues/1114

Description
This PR makes a few changes to try to help with the intermittent Stochastic Noise failures. Although as these failures are intermittent, and we haven't replicated these failures locally, these changes are speculative.

Testing:

  • Ran tests and they passed OK
  • Added new tests for the new feature(s)

@maxwhitemet
maxwhitemet force-pushed the mobt_1114_stochastic_noise_failure branch from 94c38d9 to 3a85788 Compare April 16, 2026 11:06
@maxwhitemet
maxwhitemet marked this pull request as ready for review April 16, 2026 16:13
maxwhitemet
maxwhitemet previously approved these changes Apr 16, 2026

@maxwhitemet maxwhitemet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Gavin. Happy with the changes you have made.

I have made some PR-hosted changes to address some sources of np.nan.

@mo-jbeaver mo-jbeaver left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All the tests ran successfully. I've commented on a few things, but potentially a second review of the code maybe beneficial.

Comment thread improver/calibration/stochastic_noise.py
Comment thread improver/calibration/stochastic_noise.py Outdated
Comment thread improver/cli/stochastic_noise.py
@maxwhitemet
maxwhitemet force-pushed the mobt_1114_stochastic_noise_failure branch from a94dbee to 26873d0 Compare April 21, 2026 11:10

@maxwhitemet maxwhitemet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @mo-jbeaver. I have implemented the suggested changes

Comment thread improver/calibration/stochastic_noise.py Outdated
Comment thread improver/calibration/stochastic_noise.py
Comment thread improver/cli/stochastic_noise.py
mo-jbeaver
mo-jbeaver previously approved these changes Apr 21, 2026

@mo-jbeaver mo-jbeaver left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Happy with the change changes made and all tests ran successfully.

@gavinevans gavinevans self-assigned this Apr 22, 2026

@gavinevans gavinevans left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @maxwhitemet 👍

I've added a couple of minor suggestions.

Comment thread improver/calibration/stochastic_noise.py Outdated
if not np.all(np.isfinite(result_db)):
# Repeat scaling from _to_dB to get a sub-threshold dB value for
# non-finite outputs.
sub_threshold_dB = 10.0 * np.log10(self.db_threshold) - 5

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ideally, it might be good to do something like set self.arbitrary_offset = 5 in the __init__ method, and then use self.arbitrary_offset instead of 5 on this line, and also in the _to_dB method. Perhaps that would make 5 seem less like a magic number.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added as suggested.

@gavinevans gavinevans removed their assignment Apr 22, 2026

@maxwhitemet maxwhitemet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @gavinevans. I have implemented your feedback.

Comment thread improver/calibration/stochastic_noise.py Outdated
if not np.all(np.isfinite(result_db)):
# Repeat scaling from _to_dB to get a sub-threshold dB value for
# non-finite outputs.
sub_threshold_dB = 10.0 * np.log10(self.db_threshold) - 5

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Added as suggested.

@maxwhitemet maxwhitemet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Happy with the changes made. Approved 👍

@maxwhitemet
maxwhitemet merged commit 71e2bf4 into metoppv:master Apr 27, 2026
7 checks passed
@gavinevans
gavinevans deleted the mobt_1114_stochastic_noise_failure branch April 27, 2026 15:04
MoseleyS added a commit that referenced this pull request May 14, 2026
* master:
  Changed implementation from clipping to masking + added Unit Tests (#2366)
  Adding kwarg as CLI argument & updating acceptance tests. (#2377)
  Refactor Pollen index for daily and hourly to single plugin (#2372)
  EPPT-3259 fix fsi duplicate metadata (#2370)
  Changes to Pollen classes for refactoring cube long names and units of concentration (#2368)
  Eppt 3223 lifted index investigate why the values are wrong (#2365)
  Cast to the original dtype of the points in `expand_bounds`  (#2367)
  Changes that might help with intermittent Stochastic Noise failure (#2346)
MoseleyS added a commit that referenced this pull request May 15, 2026
* master:
  MOBT-1148 Add subperiod-selector tool (#2373)
  Revert "change ApplyDecisionTree categorical cube dtype from int32 to int16 (…" (#2382)
  change ApplyDecisionTree categorical cube dtype from int32 to int16 (#2371)
  Changed implementation from clipping to masking + added Unit Tests (#2366)
  Adding kwarg as CLI argument & updating acceptance tests. (#2377)
  Refactor Pollen index for daily and hourly to single plugin (#2372)
  EPPT-3259 fix fsi duplicate metadata (#2370)
  Changes to Pollen classes for refactoring cube long names and units of concentration (#2368)
  Eppt 3223 lifted index investigate why the values are wrong (#2365)
  Cast to the original dtype of the points in `expand_bounds`  (#2367)
  Changes that might help with intermittent Stochastic Noise failure (#2346)

# Conflicts:
#	improver/categorical/subperiod_selector.py
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