Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions schemas/input/assets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ fields:
type: number
description: The capacity of the asset
notes: Must be >0
- name: num_units
type: integer
description: The number of units comprising the asset
notes: |
Optional. For processes with a `unit_size`, if omitted it is calculated by rounding the
capacity up to the nearest whole unit. For processes without a `unit_size`, if provided it
determines the unit size; otherwise the asset is represented as one unit. Must be >0.
Comment on lines +18 to +24

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not entirely sure about the purpose of this parameter, to be honest. If a process has unit_size, the number of units can be calculated out of it, as described. And if not, the number of units is one, also as described. So, why giving the option of indicating the number of units manually? It seems like adding another way of confusing the user.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agree it's confusing but it's what Adam wants. Two reasons:

  • For processes with a defined unit_size, relying on unit_size here requires capacities to be an exact multiple of unit_size, otherwise the data is incoherent (capacities will be rounded up so that everything works, but this fundamentally changes the system so should be avoided). Defining num_units here allows the data to be coherent without changing overall capacities
  • For processes without a defined unit_size, num_units=1 is just a default (which we may remove in the future). Specifying num_units allows these assets to be "divisible", in the sense that they do not have to be decommissioned in one go (which was not possible before)

Perhaps will make more sense in the context of #1483, which also clarifies the documentation a bit.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, I see the second point. But the first one feels a bit dodgy.

Let's take the usual example of wind turbines. You have that each turbine has 10 kW of capacity (a defined process with specific costs, etc). A user might ask for 45 kW of capacity, which means 5 turbines - and hence an actual capacity of 50 kW. If the user then says I want 9 units because of reasons, that results in a unit size of 5 kW, which is not what the process represents. I feel this is way less realistic that rounding up to the closest whole number of units...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't disagree, but at least now we're giving the user a choice between two imperfect solutions 🤷

- name: commission_year
type: integer
description: The year in which to commission this asset
Expand Down
41 changes: 28 additions & 13 deletions src/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ impl Asset {
);
self.capacity().assert_same_type(capacity);
assert!(
self.get_num_mothballed_units() <= capacity.n_units().unwrap_or(1),
self.get_num_mothballed_units() <= capacity.num_units().unwrap_or(1),
"Cannot set capacity to a smaller number of units than are currently mothballed"
);

Expand Down Expand Up @@ -802,7 +802,7 @@ impl Asset {
///
/// If divisible, returns the total number of units, otherwise returns one.
pub fn num_units(&self) -> u32 {
self.capacity().n_units().unwrap_or(1)
self.capacity().num_units().unwrap_or(1)
}

/// Get the unit size for this asset's capacity (if any)
Expand Down Expand Up @@ -881,25 +881,24 @@ pub fn check_region_year_valid_for_process(
pub struct UserAsset(#[deref(forward)] AssetRef);

impl UserAsset {
/// Create a new [`UserAsset`]
/// Create a new [`UserAsset`] with an explicit capacity representation.
pub fn new(
agent_id: AgentID,
process: Arc<Process>,
region_id: RegionID,
capacity: Capacity,
capacity: AssetCapacity,
commission_year: u32,
max_decommission_year: Option<u32>,
) -> Result<Self> {
check_capacity_valid_for_asset(capacity)?;
let unit_size = process.unit_size;
check_capacity_valid_for_asset(capacity.total_capacity())?;
let asset = Asset::new_with_state(
AssetState::Ready {
agent_id,
commission_reason: "user input",
},
process,
region_id,
AssetCapacity::from_capacity(capacity, unit_size),
capacity,
commission_year,
max_decommission_year,
)?;
Expand Down Expand Up @@ -1330,8 +1329,16 @@ mod tests {
region_id: RegionID,
#[case] capacity: Capacity,
) {
let asset =
UserAsset::new(agent_id, process.into(), region_id, capacity, 2015, None).unwrap();
let asset_capacity = AssetCapacity::Discrete(1, capacity);
let asset = UserAsset::new(
agent_id,
process.into(),
region_id,
asset_capacity,
2015,
None,
)
.unwrap();
assert!(asset.id().is_none());
}

Expand All @@ -1348,8 +1355,16 @@ mod tests {
region_id: RegionID,
#[case] capacity: Capacity,
) {
let asset_capacity = AssetCapacity::Discrete(1, capacity);
assert_error!(
UserAsset::new(agent_id, process.into(), region_id, capacity, 2015, None),
UserAsset::new(
agent_id,
process.into(),
region_id,
asset_capacity,
2015,
None
),
"Capacity must be a finite, positive number"
);
}
Expand All @@ -1365,7 +1380,7 @@ mod tests {
agent_id,
process.into(),
region_id,
Capacity(1.0),
AssetCapacity::Discrete(1, Capacity(1.0)),
2007,
None
),
Expand All @@ -1381,7 +1396,7 @@ mod tests {
agent_id,
process.into(),
region_id,
Capacity(1.0),
AssetCapacity::Discrete(1, Capacity(1.0)),
2015,
None
),
Expand All @@ -1404,7 +1419,7 @@ mod tests {
asset_subset.capacity(),
AssetCapacity::Discrete(num_units, Capacity(4.0))
);
assert_eq!(asset_subset.capacity().n_units(), Some(num_units));
assert_eq!(asset_subset.capacity().num_units(), Some(num_units));
assert_eq!(asset_subset.id(), asset.id());
assert_eq!(asset_subset.agent_id(), asset.agent_id());
assert_eq!(Arc::ptr_eq(&asset_subset.0, &asset.0), expect_same_asset);
Expand Down
6 changes: 3 additions & 3 deletions src/asset/capacity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl AssetCapacity {
}

/// Returns the number of units if this is a discrete capacity, or `None` if continuous.
pub fn n_units(&self) -> Option<u32> {
pub fn num_units(&self) -> Option<u32> {
match self {
AssetCapacity::Continuous(_) => None,
AssetCapacity::Discrete(units, _) => Some(*units),
Expand Down Expand Up @@ -169,7 +169,7 @@ mod tests {
#[case] expected_total: Capacity,
) {
let got = AssetCapacity::from_capacity(capacity, unit_size);
assert_eq!(got.n_units(), expected_n);
assert_eq!(got.num_units(), expected_n);
assert_eq!(got.total_capacity(), expected_total);
}

Expand All @@ -190,7 +190,7 @@ mod tests {
#[case] expected_total: Capacity,
) {
let got = AssetCapacity::from_capacity_floor(capacity, unit_size);
assert_eq!(got.n_units(), expected_n);
assert_eq!(got.num_units(), expected_n);
assert_eq!(got.total_capacity(), expected_total);
}

Expand Down
4 changes: 2 additions & 2 deletions src/asset/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ impl AssetPool {
mod tests {
use super::super::Asset;
use super::*;
use crate::asset::MothballEvent;
use crate::asset::{AssetCapacity, MothballEvent};
use crate::fixture::{asset, asset_divisible, process, process_parameter_map};
use crate::process::{Process, ProcessParameter};
use crate::units::{
Expand Down Expand Up @@ -219,7 +219,7 @@ mod tests {
"agent1".into(),
Arc::clone(&rc_process),
"GBR".into(),
Capacity(1.0),
AssetCapacity::Discrete(1, Capacity(1.0)),
year,
None,
)
Expand Down
98 changes: 87 additions & 11 deletions src/input/asset.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Code for reading user assets from a CSV file.
use super::{input_err_msg, read_csv_optional};
use crate::agent::AgentID;
use crate::asset::UserAsset;
use crate::asset::{AssetCapacity, UserAsset};
use crate::id::{GetIDValue, IDCollection};
use crate::process::ProcessMap;
use crate::region::RegionID;
Expand All @@ -23,6 +23,8 @@ struct AssetRaw {
region_id: String,
agent_id: String,
capacity: Capacity,
#[serde(default)]
num_units: Option<u32>,
commission_year: u32,
#[serde(default)]
max_decommission_year: Option<u32>,
Expand Down Expand Up @@ -104,29 +106,44 @@ where
asset.agent_id,
);

// Check that capacity is approximately a multiple of the process unit size
// If not, raise a warning
if let Some(unit_size) = process.unit_size {
// Split overall capacity into units
let asset_capacity = if let Some(num_units) = asset.num_units {
// A provided unit count takes precedence over the process unit_size.
ensure!(num_units > 0, "num_units must be positive");
Comment thread
tsmbland marked this conversation as resolved.

let unit_size = Capacity(asset.capacity.value() / num_units as f64);
AssetCapacity::Discrete(num_units, unit_size)
} else if let Some(unit_size) = process.unit_size {
// No unit count was provided, so use the process unit_size to determine
// how many units are needed to cover the asset's capacity.
let ratio = (asset.capacity / unit_size).value();
let num_units = ratio.ceil();

// Rounding up can increase the combined capacity of the resulting units.
if !approx_eq!(f64, ratio, ratio.ceil()) {
Comment thread
tsmbland marked this conversation as resolved.
let n_units = ratio.ceil();
warn!(
"Asset capacity {} for process {} is not a multiple of unit size {}. \
Asset will be divided into {} units with combined capacity of {}.",
Asset will be divided into {} units with combined capacity of {}.",
asset.capacity,
process_id,
unit_size,
n_units,
unit_size.value() * n_units
num_units,
unit_size.value() * num_units
);
}
}

#[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
AssetCapacity::Discrete(num_units as u32, unit_size)
} else {
// Without a process unit_size, lack of num_units implies the asset is indivisible.
AssetCapacity::Discrete(1, asset.capacity)
};

UserAsset::new(
agent_id.clone(),
Arc::clone(process),
region_id.clone(),
asset.capacity,
asset_capacity,
asset.commission_year,
asset.max_decommission_year,
)
Expand Down Expand Up @@ -162,14 +179,15 @@ mod tests {
process_id: "process1".into(),
region_id: "GBR".into(),
capacity: Capacity(1.0),
num_units: Some(1),
commission_year: 2010,
max_decommission_year,
};
let asset_out = UserAsset::new(
"agent1".into(),
Arc::clone(processes.values().next().unwrap()),
"GBR".into(),
Capacity(1.0),
AssetCapacity::Discrete(1, Capacity(1.0)),
2010,
max_decommission_year,
)
Expand All @@ -181,12 +199,66 @@ mod tests {
);
}

#[rstest]
fn explicit_unit_count_sets_unit_size(
agent_ids: IndexSet<AgentID>,
processes: ProcessMap,
region_ids: IndexSet<RegionID>,
) {
let asset = AssetRaw {
process_id: "process1".into(),
region_id: "GBR".into(),
agent_id: "agent1".into(),
capacity: Capacity(6.0),
num_units: Some(3),
commission_year: 2010,
max_decommission_year: None,
};

let assets =
read_assets_from_iter(iter::once(asset), &agent_ids, &processes, &region_ids).unwrap();

assert_eq!(
assets[0].capacity(),
AssetCapacity::Discrete(3, Capacity(2.0))
);
}

#[rstest]
fn missing_unit_count_uses_process_unit_size(
agent_ids: IndexSet<AgentID>,
mut processes: ProcessMap,
region_ids: IndexSet<RegionID>,
) {
Arc::get_mut(processes.get_mut("process1").unwrap())
.unwrap()
.unit_size = Some(Capacity(4.0));
let asset = AssetRaw {
process_id: "process1".into(),
region_id: "GBR".into(),
agent_id: "agent1".into(),
capacity: Capacity(9.0),
num_units: None,
commission_year: 2010,
max_decommission_year: None,
};

let assets =
read_assets_from_iter(iter::once(asset), &agent_ids, &processes, &region_ids).unwrap();

assert_eq!(
assets[0].capacity(),
AssetCapacity::Discrete(3, Capacity(4.0))
);
}

#[rstest]
#[case(AssetRaw { // Bad process ID
agent_id: "agent1".into(),
process_id: "process2".into(),
region_id: "GBR".into(),
capacity: Capacity(1.0),
num_units: None,
commission_year: 2010,
max_decommission_year: None,
})]
Expand All @@ -195,6 +267,7 @@ mod tests {
process_id: "process1".into(),
region_id: "GBR".into(),
capacity: Capacity(1.0),
num_units: None,
commission_year: 2010,
max_decommission_year: None,
})]
Expand All @@ -203,6 +276,7 @@ mod tests {
process_id: "process1".into(),
region_id: "FRA".into(),
capacity: Capacity(1.0),
num_units: None,
commission_year: 2010,
max_decommission_year: None,
})]
Expand All @@ -211,6 +285,7 @@ mod tests {
process_id: "process1".into(),
region_id: "GBR".into(),
capacity: Capacity(1.0),
num_units: None,
commission_year: 2010,
max_decommission_year: Some(2005),
})]
Expand All @@ -219,6 +294,7 @@ mod tests {
process_id: "process1".into(),
region_id: "GBR".into(),
capacity: Capacity(1.0),
num_units: None,
commission_year: 2010,
max_decommission_year: Some(2010),
})]
Expand Down
2 changes: 1 addition & 1 deletion src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ impl DataWriter {
milestone_year,
asset_id: asset.id().unwrap(),
capacity: asset.total_capacity(),
num_units: asset.capacity().n_units(),
num_units: asset.capacity().num_units(),
};
self.asset_capacities.serialize(row)?;
}
Expand Down
2 changes: 1 addition & 1 deletion src/simulation/optimisation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ fn add_capacity_variables(
let lower = ((1.0 - capacity_margin) * units as f64).max(0.0);
let mut upper = (1.0 + capacity_margin) * units as f64;
if let Some(limit) = capacity_limit {
upper = upper.min(limit.n_units().unwrap() as f64);
upper = upper.min(limit.num_units().unwrap() as f64);
}
problem.add_integer_column((coeff * unit_size).value(), lower..=upper)
}
Expand Down
Loading
Loading