Skip to content

Clamping function#1011

Merged
vgro merged 1 commit into959-sort-out-time-interval-in-energy-balancefrom
959-clamping
Sep 1, 2025
Merged

Clamping function#1011
vgro merged 1 commit into959-sort-out-time-interval-in-energy-balancefrom
959-clamping

Conversation

@davidorme
Copy link
Copy Markdown
Collaborator

@davidorme davidorme commented Aug 27, 2025

Description

This PR is a suggestion for a rework of the clamping values within limits in the mix_and_ventilate function in PR #988. The clamping process is reasonably complex and hard to evaluate when bundled in with the rest of the mixing and ventilation, so this PR:

  • Pulls the clamping process into it's own function to make it easier to test.
  • Adds a test that present various cases for the clamping of values within the vertical columns.
  • Replaces the calculate_excess function and its test, which are now built into the clamping function.
  • I've also tried to rework the function to increase efficiency - I don't think it can be done as a single array calculation (or if it can, it's beyond me today) - but I think we can track the redistribution of residuals across vertical layers more cleanly. We don't have to keep track on non-NaN indices if we keep track of the total residuals (values outside limits) upwards and handle how these are passed between layers with real and NaN values
  • Updates the mix_and_ventilate function to call the new clamping - tests of that function still pass.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Optimization (back-end change that speeds up the code)
  • Bug fix (non-breaking change which fixes an issue)

Key checklist

  • Make sure you've run the pre-commit checks: $ pre-commit run -a
  • All tests pass: $ poetry run pytest

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works
  • Relevant documentation reviewed and updated

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.95%. Comparing base (b954767) to head (81e0a5d).

Additional details and impacted files
@@                               Coverage Diff                                @@
##           959-sort-out-time-interval-in-energy-balance    #1011      +/-   ##
================================================================================
- Coverage                                         94.96%   94.95%   -0.01%     
================================================================================
  Files                                                80       80              
  Lines                                              6670     6662       -8     
================================================================================
- Hits                                               6334     6326       -8     
  Misses                                              336      336              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vgro vgro merged commit 4e28166 into 959-sort-out-time-interval-in-energy-balance Sep 1, 2025
13 checks passed
@vgro vgro deleted the 959-clamping branch September 1, 2025 12:44
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