Skip to content

Add helper for validating finite, non-negative inputs - #1468

Open
alexdewar wants to merge 2 commits into
mainfrom
deserialise-finite-non-negative-helper
Open

Add helper for validating finite, non-negative inputs#1468
alexdewar wants to merge 2 commits into
mainfrom
deserialise-finite-non-negative-helper

Conversation

@alexdewar

Copy link
Copy Markdown
Member

Description

There are loads of places in parameters.rs where we check that a number is finite and non-negative (I remembered this when I spotted another in #1463). Replace with a helper, deserialise_finite_non_negative, which works much like the existing deserialise_nonzero. This means we can remove duplicated code and some redundant tests, too.

Type of change

  • Bug fix (non-breaking change to fix an issue)
  • New feature (non-breaking change to add functionality)
  • Refactoring (non-breaking, non-functional change to improve maintainability)
  • Optimization (non-breaking change to speed up the code)
  • Breaking change (whatever its nature)
  • Documentation (improve or add documentation)

Key checklist

  • All tests pass: $ cargo test
  • The documentation builds and looks OK: $ cargo doc
  • Update release notes for the latest release if this PR adds a new feature or fixes a bug
    present in the previous release

Further checks

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

Copilot AI review requested due to automatic review settings July 31, 2026 16:57
@alexdewar alexdewar added this to MUSE Jul 31, 2026
@alexdewar alexdewar moved this to 👀 In review in MUSE Jul 31, 2026
@alexdewar
alexdewar force-pushed the deserialise-finite-non-negative-helper branch from 2229824 to 0736385 Compare July 31, 2026 16:58
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.80%. Comparing base (2d4fd29) to head (0736385).

Files with missing lines Patch % Lines
src/input.rs 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1468   +/-   ##
=======================================
  Coverage   89.80%   89.80%           
=======================================
  Files          60       60           
  Lines        8443     8436    -7     
  Branches     8443     8436    -7     
=======================================
- Hits         7582     7576    -6     
  Misses        544      544           
+ Partials      317      316    -1     

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI 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.

Pull request overview

This PR refactors repeated “finite and non-negative” numeric validation into a shared Serde deserialisation helper, reducing duplicated validation code and associated tests across input and model-parameter parsing.

Changes:

  • Added deserialise_finite_non_negative to centralise validation for finite, non-negative numeric inputs during deserialisation.
  • Updated ModelParameters fields to use the new helper and removed now-redundant per-field validation functions/tests.
  • Updated commodity demand CSV parsing to validate demand via the new deserialiser and removed redundant runtime checks/tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/model/parameters.rs Uses the new deserialiser for several parameters and removes duplicated validation helpers/tests.
src/input/commodity/demand.rs Moves demand non-negativity/finite validation into Serde deserialisation and drops redundant checks/tests.
src/input.rs Introduces deserialise_finite_non_negative plus a focused unit test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/model/parameters.rs
Ok(())
}

/// Check that the `remaining_demand_absolute_tolerance` parameter is valid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

2 participants