num_units column in assets.csv - #1480
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an explicit num_units field for user-provided assets to support making all assets.csv assets representable as discrete “units”, as an incremental step toward making all assets divisible (Issue #1441). It also standardises naming by renaming AssetCapacity::n_units() to AssetCapacity::num_units() and updates call sites accordingly.
Changes:
- Add optional
num_unitsparsing fromassets.csvand derive discreteAssetCapacityfrom either explicitnum_units, processunit_size, or default to a single unit. - Rename
AssetCapacity::n_units()→AssetCapacity::num_units()and update usages in optimisation/output/asset logic. - Update the input schema (
schemas/input/assets.yaml) to document the newnum_unitscolumn.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/simulation/optimisation.rs | Updates capacity constraint logic to use AssetCapacity::num_units(). |
| src/output.rs | Outputs num_units via the renamed AssetCapacity::num_units() accessor. |
| src/input/asset.rs | Adds num_units CSV field and constructs discrete AssetCapacity for user assets based on num_units/unit_size. |
| src/asset/pool.rs | Updates tests/construction to pass explicit AssetCapacity for UserAsset::new. |
| src/asset/capacity.rs | Renames n_units() to num_units() and updates related tests. |
| src/asset.rs | Changes UserAsset::new to accept an explicit AssetCapacity and updates internal/tests accordingly. |
| schemas/input/assets.yaml | Documents the new optional num_units column for assets.csv. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
5fb147d to
02d9e30
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1480 +/- ##
=======================================
Coverage 89.91% 89.91%
=======================================
Files 60 60
Lines 8556 8560 +4
Branches 8556 8560 +4
=======================================
+ Hits 7693 7697 +4
Misses 543 543
Partials 320 320 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
db5b58e to
6cef7f2
Compare
Description
This is the first step on the way to making all assets divisible (#1441). This PR makes all assets provided in
assets.csvdivisible (changing capacity fromContinuoustoDiscrete). Since not all processes have a definedunit_size(and we want to keep this optional), we need an extra column (num_units) specifying how many units to split the capacity into. For processes with a definedunit_size,num_unitstakes precedence. Ifn_unitsis not specified, the behaviour is essentially equivalent to how it currently is: split processes withunit_sizeaccording to this, and processes withoutn_unitsremain non-divisible (i.e. discrete capacity with a single unit).This does affect the output files, but only because it changes the
num_unitscolumn inasset_capacities.csv. Total capacities, flows and prices should be unchanged.Eventually I want to drop
AssetCapacity::Continuousentirely, but it's currently also used by assets invested in by MUSE. This is the next thing to change (#1483)This PR doesn't make much sense to merge without #1483, but I still figured I'd keep it separate to make it slightly easier to review (#1483 is pretty big so anything to break it up).
Fixes # (issue)
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks