Skip to content
36 changes: 32 additions & 4 deletions src/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,11 +831,42 @@ impl Asset {
*mothballed_year
}

/// Get the unit size for this asset's process (if any)
pub fn unit_size(&self) -> Option<Capacity> {
self.process.unit_size
}

/// Checks if the asset corresponds to a process that has a `unit_size` and is therefore divisible.
pub fn is_divisible(&self) -> bool {
self.process.unit_size.is_some()
}

/// Convert a capacity to number of units for a divisible asset.
///
/// Divides the given capacity by the process unit size and rounds up. In other words, this is
/// the minimum number of units required to achieve at least the given capacity.
///
/// Panics if the asset is not divisible.
pub fn capacity_to_units(&self, capacity: Capacity) -> u32 {
let unit_size = self.unit_size().expect("Asset must be divisible");
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
{
(capacity / unit_size).value().ceil() as u32
}
}
Comment on lines +850 to +856
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The new method capacity_to_units() lacks test coverage. Consider adding unit tests to verify correct behavior for edge cases like exact multiples, rounding up, and values smaller than unit_size.

Copilot uses AI. Check for mistakes.

/// Round a capacity up to the nearest multiple of the unit size.
///
/// For a divisible asset, returns the minimum capacity (as a multiple of `unit_size`)
/// that is at least as large as the given capacity.
///
/// Panics if the asset is not divisible.
pub fn round_capacity_to_unit_size(&self, capacity: Capacity) -> Capacity {
let unit_size = self.unit_size().expect("Asset must be divisible");
let n_units = self.capacity_to_units(capacity);
Capacity(unit_size.value() * n_units as f64)
}
Comment on lines +864 to +868
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The new method round_capacity_to_unit_size() lacks test coverage. Consider adding unit tests to verify correct rounding behavior for various capacity and unit_size combinations.

Copilot uses AI. Check for mistakes.

/// Divides an asset if it is divisible and returns a vector of children
///
/// The child assets are identical to the parent (including state) but with a capacity
Expand All @@ -860,10 +891,7 @@ impl Asset {
);

// Calculate the number of units corresponding to the asset's capacity
// Safe because capacity and unit_size are both positive finite numbers, so their ratio
// must also be positive and finite.
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
let n_units = (self.capacity / unit_size).value().ceil() as usize;
let n_units = self.capacity_to_units(self.capacity) as usize;

// Divide the asset into `n_units` children of size `unit_size`
let child_asset = Self {
Expand Down
24 changes: 17 additions & 7 deletions src/simulation/investment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,12 +617,16 @@ fn get_candidate_assets<'a>(
let mut asset =
Asset::new_candidate(process.clone(), region_id.clone(), Capacity(0.0), year)
.unwrap();
asset.set_capacity(get_demand_limiting_capacity(
time_slice_info,
&asset,
commodity,
demand,
));

// Set capacity based on demand
// This will serve as the upper limit when appraising the asset
// If the asset is divisible, round capacity to the nearest multiple of the unit size
let mut capacity =
get_demand_limiting_capacity(time_slice_info, &asset, commodity, demand);
if asset.is_divisible() {
capacity = asset.round_capacity_to_unit_size(capacity);
}
asset.set_capacity(capacity);

asset.into()
})
Expand Down Expand Up @@ -708,7 +712,13 @@ fn select_best_assets(
let mut outputs_for_opts = Vec::new();
for asset in &opt_assets {
let max_capacity = (!asset.is_commissioned()).then(|| {
let max_capacity = model.parameters.capacity_limit_factor * asset.capacity();
let mut max_capacity = model.parameters.capacity_limit_factor * asset.capacity();

// For divisible assets, round up to the nearest multiple of the process unit size
if asset.is_divisible() {
max_capacity = asset.round_capacity_to_unit_size(max_capacity);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This threw me when I first read it because I assumed it was rounding the capacity of the asset and didn't notice there was an extra argument.

I'm wondering if it might be clearer to have round_capacity_to_unit_size be a standalone function in this file instead. Also, it could handle the case of non-divisible assets by just returning the capacity argument rather than panicking. I think this might be more readable.

This is obvs a bit subjective, so up to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to get rid of this in #1079 (if we agree to go down that route), so I'll leave it for now


let remaining_capacity = remaining_candidate_capacity[asset];
max_capacity.min(remaining_capacity)
});
Expand Down
29 changes: 24 additions & 5 deletions src/simulation/investment/appraisal/constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::asset::{AssetRef, AssetState};
use crate::commodity::Commodity;
use crate::time_slice::{TimeSliceID, TimeSliceInfo};
use crate::units::{Capacity, Flow};
use float_cmp::approx_eq;
use highs::RowProblem as Problem;
use indexmap::IndexMap;

Expand All @@ -19,15 +20,26 @@ pub fn add_capacity_constraint(
max_capacity: Option<Capacity>,
capacity_var: Variable,
) {
let capacity = max_capacity.unwrap_or(asset.capacity());
let mut capacity_limit = max_capacity.unwrap_or(asset.capacity()).value();

// If asset is divisible, capacity_var represents number of units, so we must divide the
// capacity bounds by the unit size.
if let Some(unit_size) = asset.unit_size() {
capacity_limit /= unit_size.value();

// Sanity check: capacity_limit should be a whole number of units (i.e pre-adjusted
// capacity limit was a multiple of unit size)
assert!(approx_eq!(f64, capacity_limit, capacity_limit.round()));
Comment on lines +30 to +32
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

This assertion can fail for commissioned assets with divisible processes whose capacity is not an exact multiple of the unit size. The code in src/input/asset.rs allows such assets to be loaded (with a warning), but when these assets are appraised, this assertion will panic. For commissioned assets, the capacity should be used as-is without requiring it to be a perfect multiple of unit_size, or the capacity should be rounded to the nearest unit count before the assertion.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. Assets in the input will get split into units by divide_asset prior to reaching this point

}

let bounds = match asset.state() {
AssetState::Commissioned { .. } => {
// Fixed capacity for commissioned assets
capacity.value()..=capacity.value()
capacity_limit..=capacity_limit
}
AssetState::Candidate => {
// Variable capacity between 0 and max for candidate assets
0.0..=capacity.value()
0.0..=capacity_limit
}
_ => panic!(
"add_capacity_constraint should only be called with Commissioned or Candidate assets"
Expand Down Expand Up @@ -100,8 +112,15 @@ fn add_activity_constraints_for_candidate(
time_slice_info: &TimeSliceInfo,
) {
for (ts_selection, limits) in asset.iter_activity_per_capacity_limits() {
let upper_limit = limits.end().value();
let lower_limit = limits.start().value();
let mut upper_limit = limits.end().value();
let mut lower_limit = limits.start().value();

// If the asset is divisible, the capacity variable represents number of units,
// so we need to multiply the per-capacity limits by the unit size.
if let Some(unit_size) = asset.unit_size() {
upper_limit *= unit_size.value();
lower_limit *= unit_size.value();
}

// Collect capacity and activity terms
// We have a single capacity term, and activity terms for all time slices in the selection
Expand Down
33 changes: 27 additions & 6 deletions src/simulation/investment/appraisal/optimisation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ pub type Variable = highs::Col;

/// Map storing variables for the optimisation problem
struct VariableMap {
/// Capacity variable
/// Capacity variable.
///
/// This represents absolute capacity for indivisible assets and number of units for
/// divisible assets.
capacity_var: Variable,
/// Activity variables in each time slice
activity_vars: IndexMap<TimeSliceID, Variable>,
Expand All @@ -38,10 +41,23 @@ impl VariableMap {
///
/// # Returns
/// A new `VariableMap` containing all created decision variables
fn add_to_problem(problem: &mut Problem, cost_coefficients: &ObjectiveCoefficients) -> Self {
fn add_to_problem(
problem: &mut Problem,
cost_coefficients: &ObjectiveCoefficients,
capacity_unit_size: Option<Capacity>,
) -> Self {
// Create capacity variable with its associated cost
let capacity_var =
problem.add_column(cost_coefficients.capacity_coefficient.value(), 0.0..);
let capacity_coefficient = cost_coefficients.capacity_coefficient.value();
let capacity_var = match capacity_unit_size {
Some(unit_size) => {
// Divisible asset: capacity variable represents number of units
problem.add_integer_column(capacity_coefficient * unit_size.value(), 0.0..)
}
None => {
// Indivisible asset: capacity variable represents total capacity
problem.add_column(capacity_coefficient, 0.0..)
}
};

// Create activity variables for each time slice
let mut activity_vars = IndexMap::new();
Expand Down Expand Up @@ -118,7 +134,7 @@ pub fn perform_optimisation(
) -> Result<ResultsMap> {
// Create problem and add variables
let mut problem = Problem::default();
let variables = VariableMap::add_to_problem(&mut problem, coefficients);
let variables = VariableMap::add_to_problem(&mut problem, coefficients, asset.unit_size());

// Add constraints
add_constraints(
Expand All @@ -135,7 +151,12 @@ pub fn perform_optimisation(
let solution = solve_optimal(problem.optimise(sense))?.get_solution();
let solution_values = solution.columns();
Ok(ResultsMap {
capacity: Capacity::new(solution_values[0]),
// If the asset is divisible, the capacity variable represents number of units, so convert
// to total capacity
capacity: match asset.unit_size() {
Some(unit_size) => Capacity::new(solution_values[0] * unit_size.value()),
None => Capacity::new(solution_values[0]),
},
// The mapping below assumes the column ordering documented on `VariableMap::add_to_problem`:
// index 0 = capacity, next `n` entries = activities (in the same key order as
// `cost_coefficients.activity_coefficients`), remaining entries = unmet demand.
Expand Down
16 changes: 8 additions & 8 deletions tests/data/simple_divisible/assets.csv
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
asset_id,process_id,region_id,agent_id,group_id,commission_year,decommission_year,capacity
0,GASDRV,GBR,A0_GEX,,2020,,4002.26
1,GASPRC,GBR,A0_GPR,,2020,,3782.13
2,WNDFRM,GBR,A0_ELC,,2020,2040,3.964844
3,GASCGT,GBR,A0_ELC,,2020,2040,2.43
2,WNDFRM,GBR,A0_ELC,,2020,,3.964844
3,GASCGT,GBR,A0_ELC,,2020,,2.43
Comment on lines +4 to +5
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The removal of decommission years (2040) for assets 2 and 3 is not explained in the PR description. If this change is intentional and related to the integer variable modifications, it should be documented; otherwise, it may be an unintended side effect.

Suggested change
2,WNDFRM,GBR,A0_ELC,,2020,,3.964844
3,GASCGT,GBR,A0_ELC,,2020,,2.43
2,WNDFRM,GBR,A0_ELC,,2020,2040,3.964844
3,GASCGT,GBR,A0_ELC,,2020,2040,2.43

Copilot uses AI. Check for mistakes.
4,RGASBR,GBR,A0_RES,,2020,2035,1000.0
5,RGASBR,GBR,A0_RES,,2020,2035,1000.0
6,RGASBR,GBR,A0_RES,,2020,2035,1000.0
7,RELCHP,GBR,A0_RES,,2020,2035,399.98
8,RGASBR,GBR,A0_RES,1,2030,,1000.0
9,GASCGT,GBR,A0_ELC,,2030,2040,0.44245235762867363
10,RGASBR,GBR,A0_RES,2,2040,,1000.0
11,RGASBR,GBR,A0_RES,2,2040,,1000.0
12,RGASBR,GBR,A0_RES,2,2040,,1000.0
13,RGASBR,GBR,A0_RES,2,2040,,1000.0
8,RELCHP,GBR,A0_RES,,2030,,255.83840587648046
9,GASCGT,GBR,A0_ELC,,2030,2040,3.1192651014219064
Comment on lines +10 to +11
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

Assets 8 and 9 show updated capacities and different asset types/IDs compared to the original data. The change from RGASBR to RELCHP for asset 8 and the significant capacity changes (1000.0 to 255.83840587648046) should be verified as intentional outcomes of the integer optimization.

Copilot uses AI. Check for mistakes.
10,RGASBR,GBR,A0_RES,1,2040,,1000.0
11,RGASBR,GBR,A0_RES,1,2040,,1000.0
12,RGASBR,GBR,A0_RES,1,2040,,1000.0
13,RGASBR,GBR,A0_RES,1,2040,,1000.0
Loading