From 7852273734b8d1543b94473f4d7b4c2bbee4bcd6 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 14:16:20 +0000 Subject: [PATCH 01/11] Add missing Arguments header to doc comment --- src/simulation/investment.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 542f643d..071c4913 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -56,6 +56,8 @@ impl InvestmentSet { /// Selects assets for this investment set variant and passes through the shared /// context needed by single-market, cycle, or layered selection. /// + /// # Arguments + /// /// * `model` – Simulation model supplying parameters, processes, and dispatch. /// * `year` – Planning year being solved. /// * `demand` – Net demand profiles available to all markets before selection. From 0be01515d364bd05eb0c1ce99034846e88869ba1 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 14:38:31 +0000 Subject: [PATCH 02/11] Tidying --- src/simulation/investment.rs | 42 ++++++++++++++------------ src/simulation/investment/appraisal.rs | 4 +-- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 071c4913..b1f53b41 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -718,29 +718,29 @@ fn select_best_assets( )?; // Sort assets by appraisal metric - let assets_sorted_by_metric: Vec<_> = outputs_for_opts + let assets_sorted_by_metric = outputs_for_opts .into_iter() .filter(|output| output.capacity > Capacity(0.0)) .sorted_by(AppraisalOutput::compare_metric) - .collect(); - - // check if all options have zero capacity - if assets_sorted_by_metric.is_empty() { - // In this case, we cannot meet demand, so have to bail out. - // This may happen if: - // - the asset has zero activity limits for all time slices with demand. - // - known issue with the NPV objective - // (see https://github.com/EnergySystemsModellingLab/MUSE2/issues/716). - bail!( - "No feasible investment options for commodity '{}' after appraisal", - &commodity.id - ) - } - - let appraisal_comparison_method = classify_appraisal_comparison_method( - &assets_sorted_by_metric.iter().collect::>(), + .collect_vec(); + + // Check if all options have zero capacity. If so, we cannot meet demand, so have to bail + // out. + // + // This may happen if: + // - the asset has zero activity limits for all time slices with + // demand. + // - known issue with the NPV objective + // (see https://github.com/EnergySystemsModellingLab/MUSE2/issues/716). + ensure!( + !assets_sorted_by_metric.is_empty(), + "No feasible investment options for commodity '{}' after appraisal", + &commodity.id ); + let appraisal_comparison_method = + classify_appraisal_comparison_method(&assets_sorted_by_metric); + // Determine the best asset based on whether multiple equally-good options exist let best_output = match appraisal_comparison_method { // there are multiple equally good assets by metric @@ -754,8 +754,10 @@ fn select_best_assets( .count(); // select from all equally good assets - let equally_good_assets: Vec<_> = - assets_sorted_by_metric.into_iter().take(count).collect(); + let equally_good_assets = assets_sorted_by_metric + .into_iter() + .take(count) + .collect_vec(); select_from_assets_with_equal_metric( region_id, &agent.id, diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 2493d69c..4d23bf5c 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -99,11 +99,11 @@ pub enum AppraisalComparisonMethod { /// Classify the appropriate method to compare appraisal outputs /// given an array of appraisal outputs sorted by metric pub fn classify_appraisal_comparison_method( - appraisals_sorted_by_metric: &[&AppraisalOutput], + appraisals_sorted_by_metric: &[AppraisalOutput], ) -> AppraisalComparisonMethod { if appraisals_sorted_by_metric.len() >= 2 && appraisals_sorted_by_metric[0] - .compare_metric(appraisals_sorted_by_metric[1]) + .compare_metric(&appraisals_sorted_by_metric[1]) .is_eq() { AppraisalComparisonMethod::EqualMetrics From a65961cb2e100613ac0b0d20a7b8be11c4a5932d Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 15:01:35 +0000 Subject: [PATCH 03/11] Simplify code for warning about equally good assets --- src/simulation/investment.rs | 75 +++++++++----------------- src/simulation/investment/appraisal.rs | 24 --------- 2 files changed, 26 insertions(+), 73 deletions(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index b1f53b41..97ffed4a 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -19,10 +19,7 @@ use std::fmt::Display; pub mod appraisal; use appraisal::coefficients::calculate_coefficients_for_assets; -use appraisal::{ - AppraisalComparisonMethod, AppraisalOutput, appraise_investment, - classify_appraisal_comparison_method, -}; +use appraisal::{AppraisalOutput, appraise_investment}; /// A map of demand across time slices for a specific market type DemandMap = IndexMap; @@ -623,16 +620,19 @@ fn get_candidate_assets<'a>( }) } -fn select_from_assets_with_equal_metric( - region_id: &RegionID, +/// Warn if there are multiple equally good outputs +fn warn_on_equal_appraisal_outputs( + outputs: &[AppraisalOutput], agent_id: &AgentID, commodity_id: &CommodityID, - equally_good_assets: Vec, -) -> AppraisalOutput { - // Format asset details for diagnostic logging - let asset_details = equally_good_assets + region_id: &RegionID, +) { + // Get all the outputs which are equal to the one after + let asset_details = outputs .iter() - .map(|output| { + .tuple_windows() + .take_while(|(a, b)| a.compare_metric(b).is_eq()) + .map(|(output, _)| { format!( "Process id: '{}' (State: {}{})", output.asset.process_id(), @@ -644,14 +644,14 @@ fn select_from_assets_with_equal_metric( .unwrap_or_default(), ) }) - .collect::>() .join(", "); - let warning_message = format!( - "Could not resolve deadlock between equally good appraisals for Agent id: {agent_id}, Commodity: '{commodity_id}', Region: {region_id}. Options: [{asset_details}]. Selecting first option.", - ); - warn!("{warning_message}"); - // Select the first asset arbitrarily from the equally performing options - equally_good_assets.into_iter().next().unwrap() + + if !asset_details.is_empty() { + warn!( + "Found equally good appraisals for Agent id: {agent_id}, Commodity: '{commodity_id}', \ + Region: {region_id}. Options: [{asset_details}]. Selecting first option.", + ); + } } /// Get the best assets for meeting demand for the given commodity @@ -738,38 +738,15 @@ fn select_best_assets( &commodity.id ); - let appraisal_comparison_method = - classify_appraisal_comparison_method(&assets_sorted_by_metric); + // Warn if there are multiple equally good assets + warn_on_equal_appraisal_outputs( + &assets_sorted_by_metric, + &agent.id, + &commodity.id, + region_id, + ); - // Determine the best asset based on whether multiple equally-good options exist - let best_output = match appraisal_comparison_method { - // there are multiple equally good assets by metric - AppraisalComparisonMethod::EqualMetrics => { - // Count how many assets have the same metric as the best one - let count = assets_sorted_by_metric - .iter() - .take_while(|output| { - AppraisalOutput::compare_metric(&assets_sorted_by_metric[0], output).is_eq() - }) - .count(); - - // select from all equally good assets - let equally_good_assets = assets_sorted_by_metric - .into_iter() - .take(count) - .collect_vec(); - select_from_assets_with_equal_metric( - region_id, - &agent.id, - &commodity.id, - equally_good_assets, - ) - } - // there is a single best asset by metric - AppraisalComparisonMethod::Metric => { - assets_sorted_by_metric.into_iter().next().unwrap() - } - }; + let best_output = assets_sorted_by_metric.into_iter().next().unwrap(); // Log the selected asset debug!( diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 4d23bf5c..03f09d09 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -88,30 +88,6 @@ impl AppraisalOutput { } } -/// methods used to compare multiple appraisal outputs -pub enum AppraisalComparisonMethod { - /// If all appraisal outputs have different metrics - Metric, - /// two or more appraisal outputs have equal metrics - EqualMetrics, -} - -/// Classify the appropriate method to compare appraisal outputs -/// given an array of appraisal outputs sorted by metric -pub fn classify_appraisal_comparison_method( - appraisals_sorted_by_metric: &[AppraisalOutput], -) -> AppraisalComparisonMethod { - if appraisals_sorted_by_metric.len() >= 2 - && appraisals_sorted_by_metric[0] - .compare_metric(&appraisals_sorted_by_metric[1]) - .is_eq() - { - AppraisalComparisonMethod::EqualMetrics - } else { - AppraisalComparisonMethod::Metric - } -} - /// Calculate LCOX for a hypothetical investment in the given asset. /// /// This is more commonly referred to as Levelised Cost of *Electricity*, but as the model can From 0fefe9c0114eb8ea6ae3aa794e4331504453ebc7 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 15:03:47 +0000 Subject: [PATCH 04/11] warn_on_equal_appraisal_outputs: Change `warn!` to `debug!` --- src/simulation/investment.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 97ffed4a..3c0666ad 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -13,7 +13,7 @@ use crate::units::{Capacity, Dimensionless, Flow, FlowPerCapacity}; use anyhow::{Context, Result, bail, ensure}; use indexmap::IndexMap; use itertools::{Itertools, chain}; -use log::{debug, warn}; +use log::debug; use std::collections::HashMap; use std::fmt::Display; @@ -620,7 +620,7 @@ fn get_candidate_assets<'a>( }) } -/// Warn if there are multiple equally good outputs +/// Print debug message if there are multiple equally good outputs fn warn_on_equal_appraisal_outputs( outputs: &[AppraisalOutput], agent_id: &AgentID, @@ -647,7 +647,7 @@ fn warn_on_equal_appraisal_outputs( .join(", "); if !asset_details.is_empty() { - warn!( + debug!( "Found equally good appraisals for Agent id: {agent_id}, Commodity: '{commodity_id}', \ Region: {region_id}. Options: [{asset_details}]. Selecting first option.", ); From 3a3e9a9a6983c98307e3db89e7dbe97ef225d9b3 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 15:22:02 +0000 Subject: [PATCH 05/11] appraisal.rs: Factor out comparing assets to separate function --- src/asset.rs | 26 +++++++++ src/simulation/investment/appraisal.rs | 78 ++++++++++++++++++++------ 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/src/asset.rs b/src/asset.rs index fe6e7566..58f2d42b 100644 --- a/src/asset.rs +++ b/src/asset.rs @@ -195,6 +195,32 @@ impl Asset { ) } + /// Create a new commissioned asset + /// + /// This is only used for testing. WARNING: These assets always have an ID of zero, so can + /// create hash collisions. Use with care. + #[cfg(test)] + pub fn new_commissioned( + agent_id: AgentID, + process: Rc, + region_id: RegionID, + capacity: Capacity, + commission_year: u32, + ) -> Result { + Self::new_with_state( + AssetState::Commissioned { + id: AssetID(0), + agent_id, + mothballed_year: None, + }, + process, + region_id, + capacity, + commission_year, + None, + ) + } + /// Private helper to create an asset with the given state fn new_with_state( state: AssetState, diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 03f09d09..343fc615 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -1,7 +1,7 @@ //! Calculation for investment tools such as Levelised Cost of X (LCOX) and Net Present Value (NPV). use super::DemandMap; use crate::agent::ObjectiveType; -use crate::asset::AssetRef; +use crate::asset::{Asset, AssetRef}; use crate::commodity::Commodity; use crate::finance::{lcox, profitability_index}; use crate::model::Model; @@ -67,25 +67,28 @@ impl AppraisalOutput { "Appraisal metrics must be equal" ); - // Favour commissioned assets over non-commissioned - if self.asset.is_commissioned() && !other.asset.is_commissioned() { - return Ordering::Less; - } - if !self.asset.is_commissioned() && other.asset.is_commissioned() { - return Ordering::Greater; - } + compare_asset_fallback(&self.asset, &other.asset) + } +} - // if both commissioned, favour newer ones - if self.asset.is_commissioned() && other.asset.is_commissioned() { - return self - .asset - .commission_year() - .cmp(&other.asset.commission_year()) - .reverse(); - } +fn compare_asset_fallback(asset1: &Asset, asset2: &Asset) -> Ordering { + // Favour commissioned assets over non-commissioned + if asset1.is_commissioned() && !asset2.is_commissioned() { + return Ordering::Less; + } + if !asset1.is_commissioned() && asset2.is_commissioned() { + return Ordering::Greater; + } - Ordering::Equal + // if both commissioned, favour newer ones + if asset1.is_commissioned() && asset2.is_commissioned() { + return asset1 + .commission_year() + .cmp(&asset2.commission_year()) + .reverse(); } + + Ordering::Equal } /// Calculate LCOX for a hypothetical investment in the given asset. @@ -192,3 +195,44 @@ pub fn appraise_investment( }; appraisal_method(model, asset, max_capacity, commodity, coefficients, demand) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::agent::AgentID; + use crate::asset::Asset; + use crate::fixture::{agent_id, process, region_id}; + use crate::process::Process; + use crate::region::RegionID; + use crate::units::Capacity; + use rstest::rstest; + use std::rc::Rc; + + #[rstest] + fn test_compare_assets_fallback(process: Process, region_id: RegionID, agent_id: AgentID) { + let process = Rc::new(process); + let capacity = Capacity(2.0); + let asset1 = Asset::new_commissioned( + agent_id.clone(), + process.clone(), + region_id.clone(), + capacity, + 2015, + ) + .unwrap(); + let asset2 = + Asset::new_candidate(process.clone(), region_id.clone(), capacity, 2015).unwrap(); + let asset3 = + Asset::new_commissioned(agent_id, process, region_id.clone(), capacity, 2010).unwrap(); + + assert!(compare_asset_fallback(&asset1, &asset1).is_eq()); + assert!(compare_asset_fallback(&asset2, &asset2).is_eq()); + assert!(compare_asset_fallback(&asset3, &asset3).is_eq()); + assert!(compare_asset_fallback(&asset1, &asset2).is_lt()); + assert!(compare_asset_fallback(&asset2, &asset1).is_gt()); + assert!(compare_asset_fallback(&asset1, &asset3).is_lt()); + assert!(compare_asset_fallback(&asset3, &asset1).is_gt()); + assert!(compare_asset_fallback(&asset3, &asset2).is_lt()); + assert!(compare_asset_fallback(&asset2, &asset3).is_gt()); + } +} From 3cd5896afe7dbb3417b89ecf5efd9b225b4688df Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 15:45:45 +0000 Subject: [PATCH 06/11] Simplify `compare_asset_fallback` --- src/simulation/investment/appraisal.rs | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 343fc615..f8ebb066 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -71,24 +71,10 @@ impl AppraisalOutput { } } +/// Sort commissioned assets first and newer before older fn compare_asset_fallback(asset1: &Asset, asset2: &Asset) -> Ordering { - // Favour commissioned assets over non-commissioned - if asset1.is_commissioned() && !asset2.is_commissioned() { - return Ordering::Less; - } - if !asset1.is_commissioned() && asset2.is_commissioned() { - return Ordering::Greater; - } - - // if both commissioned, favour newer ones - if asset1.is_commissioned() && asset2.is_commissioned() { - return asset1 - .commission_year() - .cmp(&asset2.commission_year()) - .reverse(); - } - - Ordering::Equal + (asset2.is_commissioned(), asset2.commission_year()) + .cmp(&(asset1.is_commissioned(), asset1.commission_year())) } /// Calculate LCOX for a hypothetical investment in the given asset. From a2d8ae730c88021a8575d5526cf7c065341607c2 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 18 Dec 2025 15:49:30 +0000 Subject: [PATCH 07/11] Move `compare_asset_fallback` to `investment.rs` --- src/simulation/investment.rs | 52 ++++++++++++++++++++-- src/simulation/investment/appraisal.rs | 61 +------------------------- 2 files changed, 50 insertions(+), 63 deletions(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 3c0666ad..7712a349 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -14,6 +14,7 @@ use anyhow::{Context, Result, bail, ensure}; use indexmap::IndexMap; use itertools::{Itertools, chain}; use log::debug; +use std::cmp::Ordering; use std::collections::HashMap; use std::fmt::Display; @@ -721,7 +722,11 @@ fn select_best_assets( let assets_sorted_by_metric = outputs_for_opts .into_iter() .filter(|output| output.capacity > Capacity(0.0)) - .sorted_by(AppraisalOutput::compare_metric) + .sorted_by(|output1, output2| match output1.compare_metric(output2) { + // If equal, we fall back on comparing asset properties + Ordering::Equal => compare_asset_fallback(&output1.asset, &output2.asset), + cmp => cmp, + }) .collect_vec(); // Check if all options have zero capacity. If so, we cannot meet demand, so have to bail @@ -787,6 +792,14 @@ fn is_any_remaining_demand(demand: &DemandMap) -> bool { demand.values().any(|flow| *flow > Flow(0.0)) } +/// Compare assets, sorting commissioned before uncommissioned and newer before older. +/// +/// Used as a fallback to sort assets when they have equal appraisal tool outputs. +fn compare_asset_fallback(asset1: &Asset, asset2: &Asset) -> Ordering { + (asset2.is_commissioned(), asset2.commission_year()) + .cmp(&(asset1.is_commissioned(), asset1.commission_year())) +} + /// Update capacity of chosen asset, if needed, and update both asset options and chosen assets fn update_assets( mut best_asset: AssetRef, @@ -833,14 +846,17 @@ fn update_assets( #[cfg(test)] mod tests { use super::*; + use crate::agent::AgentID; + use crate::asset::Asset; use crate::commodity::Commodity; use crate::fixture::{ - asset, process, process_activity_limits_map, process_flows_map, svd_commodity, time_slice, - time_slice_info, time_slice_info2, + agent_id, asset, process, process_activity_limits_map, process_flows_map, region_id, + svd_commodity, time_slice, time_slice_info, time_slice_info2, }; use crate::process::{ActivityLimits, FlowType, Process, ProcessFlow}; + use crate::region::RegionID; use crate::time_slice::{TimeSliceID, TimeSliceInfo}; - use crate::units::{Flow, FlowPerActivity, MoneyPerFlow}; + use crate::units::{Capacity, Flow, FlowPerActivity, MoneyPerFlow}; use indexmap::indexmap; use rstest::rstest; use std::rc::Rc; @@ -926,4 +942,32 @@ mod tests { // Maximum = 20.0 assert_eq!(result, Capacity(20.0)); } + + #[rstest] + fn test_compare_assets_fallback(process: Process, region_id: RegionID, agent_id: AgentID) { + let process = Rc::new(process); + let capacity = Capacity(2.0); + let asset1 = Asset::new_commissioned( + agent_id.clone(), + process.clone(), + region_id.clone(), + capacity, + 2015, + ) + .unwrap(); + let asset2 = + Asset::new_candidate(process.clone(), region_id.clone(), capacity, 2015).unwrap(); + let asset3 = + Asset::new_commissioned(agent_id, process, region_id.clone(), capacity, 2010).unwrap(); + + assert!(compare_asset_fallback(&asset1, &asset1).is_eq()); + assert!(compare_asset_fallback(&asset2, &asset2).is_eq()); + assert!(compare_asset_fallback(&asset3, &asset3).is_eq()); + assert!(compare_asset_fallback(&asset1, &asset2).is_lt()); + assert!(compare_asset_fallback(&asset2, &asset1).is_gt()); + assert!(compare_asset_fallback(&asset1, &asset3).is_lt()); + assert!(compare_asset_fallback(&asset3, &asset1).is_gt()); + assert!(compare_asset_fallback(&asset3, &asset2).is_lt()); + assert!(compare_asset_fallback(&asset2, &asset3).is_gt()); + } } diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index f8ebb066..6a250f1a 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -1,7 +1,7 @@ //! Calculation for investment tools such as Levelised Cost of X (LCOX) and Net Present Value (NPV). use super::DemandMap; use crate::agent::ObjectiveType; -use crate::asset::{Asset, AssetRef}; +use crate::asset::AssetRef; use crate::commodity::Commodity; use crate::finance::{lcox, profitability_index}; use crate::model::Model; @@ -54,27 +54,11 @@ impl AppraisalOutput { ); if approx_eq!(f64, self.metric, other.metric) { - self.compare_with_equal_metrics(other) + Ordering::Equal } else { self.metric.partial_cmp(&other.metric).unwrap() } } - - /// Compare this appraisal to another when the metrics are known to be equal. - pub fn compare_with_equal_metrics(&self, other: &Self) -> Ordering { - assert!( - approx_eq!(f64, self.metric, other.metric), - "Appraisal metrics must be equal" - ); - - compare_asset_fallback(&self.asset, &other.asset) - } -} - -/// Sort commissioned assets first and newer before older -fn compare_asset_fallback(asset1: &Asset, asset2: &Asset) -> Ordering { - (asset2.is_commissioned(), asset2.commission_year()) - .cmp(&(asset1.is_commissioned(), asset1.commission_year())) } /// Calculate LCOX for a hypothetical investment in the given asset. @@ -181,44 +165,3 @@ pub fn appraise_investment( }; appraisal_method(model, asset, max_capacity, commodity, coefficients, demand) } - -#[cfg(test)] -mod tests { - use super::*; - use crate::agent::AgentID; - use crate::asset::Asset; - use crate::fixture::{agent_id, process, region_id}; - use crate::process::Process; - use crate::region::RegionID; - use crate::units::Capacity; - use rstest::rstest; - use std::rc::Rc; - - #[rstest] - fn test_compare_assets_fallback(process: Process, region_id: RegionID, agent_id: AgentID) { - let process = Rc::new(process); - let capacity = Capacity(2.0); - let asset1 = Asset::new_commissioned( - agent_id.clone(), - process.clone(), - region_id.clone(), - capacity, - 2015, - ) - .unwrap(); - let asset2 = - Asset::new_candidate(process.clone(), region_id.clone(), capacity, 2015).unwrap(); - let asset3 = - Asset::new_commissioned(agent_id, process, region_id.clone(), capacity, 2010).unwrap(); - - assert!(compare_asset_fallback(&asset1, &asset1).is_eq()); - assert!(compare_asset_fallback(&asset2, &asset2).is_eq()); - assert!(compare_asset_fallback(&asset3, &asset3).is_eq()); - assert!(compare_asset_fallback(&asset1, &asset2).is_lt()); - assert!(compare_asset_fallback(&asset2, &asset1).is_gt()); - assert!(compare_asset_fallback(&asset1, &asset3).is_lt()); - assert!(compare_asset_fallback(&asset3, &asset1).is_gt()); - assert!(compare_asset_fallback(&asset3, &asset2).is_lt()); - assert!(compare_asset_fallback(&asset2, &asset3).is_gt()); - } -} From cbd50d2982a24e4c3eb76b3ec85a5d5529f4b756 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Fri, 19 Dec 2025 10:28:58 +0000 Subject: [PATCH 08/11] Improve doc comment for `compare_assets_fallback` Suggested by @dalonsoa. --- src/simulation/investment.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 7712a349..8a25a5e5 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -792,7 +792,9 @@ fn is_any_remaining_demand(demand: &DemandMap) -> bool { demand.values().any(|flow| *flow > Flow(0.0)) } -/// Compare assets, sorting commissioned before uncommissioned and newer before older. +/// Compare assets as a fallback if metrics are equal. +/// +/// Commissioned assets are ordered before uncommissioned and newer before older. /// /// Used as a fallback to sort assets when they have equal appraisal tool outputs. fn compare_asset_fallback(asset1: &Asset, asset2: &Asset) -> Ordering { From b1afe73b5bb14eb9a750259d4fcff70edba2baf7 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 22 Dec 2025 09:49:30 +0000 Subject: [PATCH 09/11] Also include commission year in log when warning about equal appraisal outputs --- src/simulation/investment.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 8a25a5e5..4e9a6cfd 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -634,15 +634,16 @@ fn warn_on_equal_appraisal_outputs( .tuple_windows() .take_while(|(a, b)| a.compare_metric(b).is_eq()) .map(|(output, _)| { + let asset = &output.asset; format!( - "Process id: '{}' (State: {}{})", - output.asset.process_id(), - output.asset.state(), - output - .asset + "Process id: '{}' (State: {}{}, Commission year: {})", + asset.process_id(), + asset.state(), + asset .id() .map(|id| format!(", Asset id: {id}")) .unwrap_or_default(), + asset.commission_year() ) }) .join(", "); From f2e643c32b3c26dd82f2bee0aaad28d10fdd5ca0 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 22 Dec 2025 09:53:41 +0000 Subject: [PATCH 10/11] Fix compile error --- src/asset.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/asset.rs b/src/asset.rs index 342278c0..dcc7b6ce 100644 --- a/src/asset.rs +++ b/src/asset.rs @@ -230,6 +230,7 @@ impl Asset { id: AssetID(0), agent_id, mothballed_year: None, + group_id: None, }, process, region_id, From c632d74c79fcd9289dc50581946bcee8a0761796 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 22 Dec 2025 10:15:45 +0000 Subject: [PATCH 11/11] Fix: Not printing last duplicate asset in `warn_on_equal_appraisal` --- src/simulation/investment.rs | 46 ++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 4e9a6cfd..2900ac89 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -628,29 +628,35 @@ fn warn_on_equal_appraisal_outputs( commodity_id: &CommodityID, region_id: &RegionID, ) { - // Get all the outputs which are equal to the one after - let asset_details = outputs + if outputs.is_empty() { + return; + } + + // Count the number of identical (or nearly identical) appraisal outputs + let num_identical = outputs[1..] .iter() - .tuple_windows() - .take_while(|(a, b)| a.compare_metric(b).is_eq()) - .map(|(output, _)| { - let asset = &output.asset; - format!( - "Process id: '{}' (State: {}{}, Commission year: {})", - asset.process_id(), - asset.state(), - asset - .id() - .map(|id| format!(", Asset id: {id}")) - .unwrap_or_default(), - asset.commission_year() - ) - }) - .join(", "); + .take_while(|output| outputs[0].compare_metric(output).is_eq()) + .count(); - if !asset_details.is_empty() { + if num_identical > 0 { + let asset_details = outputs[..=num_identical] + .iter() + .map(|output| { + let asset = &output.asset; + format!( + "Process ID: '{}' (State: {}{}, Commission year: {})", + asset.process_id(), + asset.state(), + asset + .id() + .map(|id| format!(", Asset ID: {id}")) + .unwrap_or_default(), + asset.commission_year() + ) + }) + .join(", "); debug!( - "Found equally good appraisals for Agent id: {agent_id}, Commodity: '{commodity_id}', \ + "Found equally good appraisals for Agent ID: {agent_id}, Commodity: '{commodity_id}', \ Region: {region_id}. Options: [{asset_details}]. Selecting first option.", ); }