Skip to content

Clean up how we specify default values for model parameters#1260

Open
alexdewar wants to merge 2 commits intomainfrom
cleaner-model.toml-defaults
Open

Clean up how we specify default values for model parameters#1260
alexdewar wants to merge 2 commits intomainfrom
cleaner-model.toml-defaults

Conversation

@alexdewar
Copy link
Copy Markdown
Collaborator

@alexdewar alexdewar commented Apr 23, 2026

Description

I started work on #420, but thought the code could use a little clean-up first, so I've opened this as a separate PR.

The way we currently define default values for model parameters (as read from model.toml) is with a couple of macros in src/model/parameters.rs, but this ends up being a bit verbose and ugly. I've cleaned it up by using a custom Default implementation for the ModelParameters struct instead, which lets us list the default values in a more natural way. The downside of this approach is that it does mean we need to define a "default" value for milestone_years, even though this parameter is not optional, then verify that a value was provided after the fact. I think it's worth it overall though.

While I was at it I changed the capacity_margin parameter to be a Dimensionless rather than an f64, to make it consistent with the other parameters. It's good to be explicit that it represents a ratio, not some physical value.

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

Let's make it consistent with other parameters. This way is less ambiguous.
Copilot AI review requested due to automatic review settings April 23, 2026 13:39
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.76%. Comparing base (a10a1f4) to head (6684455).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1260      +/-   ##
==========================================
+ Coverage   89.74%   89.76%   +0.01%     
==========================================
  Files          57       57              
  Lines        8195     8210      +15     
  Branches     8195     8210      +15     
==========================================
+ Hits         7355     7370      +15     
  Misses        544      544              
  Partials      296      296              

☔ 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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors how model.toml parameter defaults are defined by moving from per-field default macros to a Default implementation on ModelParameters, and aligns capacity_margin with the codebase’s unit-typed parameters (Dimensionless) for consistency.

Changes:

  • Replace macro-based per-field defaults in ModelParameters with #[serde(default)] + a custom Default impl, while validating required milestone_years post-deserialization.
  • Change capacity_margin from f64 to Dimensionless in model parameters and propagate the type change into dispatch optimisation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/simulation/optimisation.rs Updates DispatchRun/capacity variable plumbing to accept Dimensionless capacity_margin, converting to f64 only at the solver-boundary.
src/model/parameters.rs Introduces ModelParameters: Default to centralize defaults, removes old default macros, improves the milestone_years validation message, and makes capacity_margin unit-typed.

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

@alexdewar alexdewar added this to MUSE Apr 23, 2026
@alexdewar alexdewar moved this to 👀 In review in MUSE Apr 23, 2026
@alexdewar alexdewar force-pushed the cleaner-model.toml-defaults branch from 6a006c0 to 6684455 Compare April 23, 2026 14:05
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