Skip to content

Enable independent HC and RC in wind downscaling - #2324

Open
mo-AliceLake wants to merge 37 commits into
feature_update_wind_downscalingfrom
alice-split-winddownscaling
Open

Enable independent HC and RC in wind downscaling#2324
mo-AliceLake wants to merge 37 commits into
feature_update_wind_downscalingfrom
alice-split-winddownscaling

Conversation

@mo-AliceLake

@mo-AliceLake mo-AliceLake commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Description

This PR restructures the WindTerrainAdjustment plugin (previously RoughnessCorrection) to provide clean, explicit control over applying Roughness Correction (RC), Height Correction (HC), or their combined behaviour. The goal is to make HC‑only and RC‑only configurations explicitly selectable, allowing clearer scientific experimentation and operational configurations (e.g. HC‑only over UK-domain, etc).

Previously, HC and RC were tightly coupled within a single routine. Running only one of the corrections was possible but relied on implicit side‑effects (e.g. suppressing RC by passing z0=None), which was neither obvious nor robust. This PR replaces those implicit behaviours with a clear interface.

Previously, HC and RC were tightly coupled inside a single routine (do_rc_hc_all). This made it difficult to:

  • run HC or RC independently for testing or research,
  • apply only one component in operational setups,
  • clearly isolate the individual contribution, performance impacts, or numerical behaviour of HC vs RC.

Summary of Changes

Introduced explicit correction modes:

  • Split the existing do_rc_hc_all routine into three:
    • do_hc()
    • do_rc()
    • do_hc_and_rc() (chained, preserving legacy HC+RC behaviour)
  • Added a mode argument to the plugin constructor, allowing users to select:
    • "hc_and_rc" (default, identical to previous behaviour)
    • "hc"
    • "rc"

Clarified RC dependency on z0:
In the new interface, requesting RC (mode="rc" or "hc_and_rc") now requires a valid roughness‑length (z0) field. If it is missing, the plugin raises a clear ValueError.
This replaces the old implicit behaviour where suppressing RC was only achievable indirectly by passing z0=None. The new behaviour is explicit and predictable.

Unified masking and validation
Missing‑data, sea‑point, and invalid‑height masking has been consolidated and made consistent across RC, HC, and combined modes.

Testing

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

@mo-AliceLake
mo-AliceLake changed the base branch from master to feature_update_wind_downscaling March 10, 2026 14:12
@mo-AliceLake
mo-AliceLake force-pushed the alice-split-winddownscaling branch from 82e18ff to 41f9625 Compare March 11, 2026 16:10
@mo-AliceLake
mo-AliceLake changed the base branch from feature_update_wind_downscaling to master June 12, 2026 13:43
@mo-AliceLake
mo-AliceLake changed the base branch from master to feature_update_wind_downscaling June 12, 2026 13:43
@mo-AliceLake
mo-AliceLake marked this pull request as ready for review June 12, 2026 13:44

@brhooper brhooper 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-AliceLake , this looks to have made a lot of progress towards allowing independent application of roughness and height corrections for wind downscaling.

I've added a few comments suggesting ways in which I think the code might be improved. I haven't gotten as far as reviewing the unit tests yet, I hope to review them tomorrow. I have noted that the acceptance tests need updating following changes to the names of things.

Comment thread improver/wind_calculations/wind_downscaling.py Outdated
Comment thread improver/wind_calculations/wind_downscaling.py Outdated
Comment thread improver/wind_calculations/wind_downscaling.py Outdated
Comment thread improver/wind_calculations/wind_downscaling.py Outdated
Comment thread improver/wind_calculations/wind_downscaling.py Outdated
Comment thread improver/wind_calculations/wind_downscaling.py
Comment thread improver/wind_calculations/wind_downscaling.py
Comment thread improver/wind_calculations/wind_downscaling.py
Comment thread improver/wind_calculations/wind_downscaling.py Outdated
Comment thread improver/wind_calculations/wind_downscaling.py Outdated

@brhooper brhooper 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-AliceLake, one additional comment that I should have added yesterday. I think the unit tests look fine.

Comment thread improver/wind_calculations/wind_downscaling.py
@mo-AliceLake
mo-AliceLake requested a review from brhooper July 27, 2026 11:03

@brhooper brhooper 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-AliceLake I think that these changes have improved the code.

I think the associated CLI (improver/cli/wind_downscaling.py) still needs updating to import the new plugin name. This is currently causing some of the acceptance tests to fail.

@mo-AliceLake
mo-AliceLake requested a review from brhooper July 27, 2026 15:09
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.

2 participants