From 701ab71b7555eda38624fbeb384a806d5b515561 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Mon, 8 Dec 2025 17:11:42 +0000 Subject: [PATCH 1/7] add profitability index struct --- src/finance.rs | 25 +++++++++++++++++++++---- src/simulation/investment/appraisal.rs | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/finance.rs b/src/finance.rs index 9950b6f18..ab60faa43 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -28,13 +28,27 @@ pub fn annual_capital_cost( capital_cost * crf } +/// Represents the profitability index of an investment +/// in terms of it's numerator and denominator components. +pub struct ProfitabilityIndex { + total_annualised_surplus: Money, + annualised_fixed_cost: Money, +} + +impl ProfitabilityIndex { + /// Calculates the value of the profitability index. + pub fn value(&self) -> Dimensionless { + self.total_annualised_surplus / self.annualised_fixed_cost + } +} + /// Calculates an annual profitability index based on capacity and activity. pub fn profitability_index( capacity: Capacity, annual_fixed_cost: MoneyPerCapacity, activity: &IndexMap, activity_surpluses: &IndexMap, -) -> Dimensionless { +) -> ProfitabilityIndex { // Calculate the annualised fixed costs let annualised_fixed_cost = annual_fixed_cost * capacity; @@ -45,7 +59,10 @@ pub fn profitability_index( total_annualised_surplus += activity_surplus * *activity; } - total_annualised_surplus / annualised_fixed_cost + ProfitabilityIndex { + total_annualised_surplus, + annualised_fixed_cost, + } } /// Calculates annual LCOX based on capacity and activity. @@ -171,7 +188,7 @@ mod tests { &activity_surpluses, ); - assert_approx_eq!(Dimensionless, result, Dimensionless(expected)); + assert_approx_eq!(Dimensionless, result.value(), Dimensionless(expected)); } #[test] @@ -183,7 +200,7 @@ mod tests { let result = profitability_index(capacity, annual_fixed_cost, &activity, &activity_surpluses); - assert_eq!(result, Dimensionless(0.0)); + assert_eq!(result.value(), Dimensionless(0.0)); } #[rstest] diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 2493d69c4..3321261cb 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -194,7 +194,7 @@ fn calculate_npv( capacity: results.capacity, activity: results.activity, unmet_demand: results.unmet_demand, - metric: -profitability_index.value(), + metric: -profitability_index.value().value(), coefficients: coefficients.clone(), demand: demand.clone(), }) From f2ebcae1ce540c8b57985d04611e39aef605c20b Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 10 Dec 2025 16:18:33 +0000 Subject: [PATCH 2/7] add metric_precedence for to allow for comparing appraisals with different metrics --- src/finance.rs | 8 ++- src/fixture.rs | 1 + src/simulation/investment.rs | 3 + src/simulation/investment/appraisal.rs | 83 +++++++++++++++++++++++++- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/src/finance.rs b/src/finance.rs index ab60faa43..d0731e0ae 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -29,10 +29,12 @@ pub fn annual_capital_cost( } /// Represents the profitability index of an investment -/// in terms of it's numerator and denominator components. +/// in terms of it's annualised components. pub struct ProfitabilityIndex { - total_annualised_surplus: Money, - annualised_fixed_cost: Money, + /// the total annualised surplus of an asset + pub total_annualised_surplus: Money, + /// the total annualised fixed cost of an asset + pub annualised_fixed_cost: Money, } impl ProfitabilityIndex { diff --git a/src/fixture.rs b/src/fixture.rs index c6d5956b9..af17403cd 100644 --- a/src/fixture.rs +++ b/src/fixture.rs @@ -304,6 +304,7 @@ pub fn appraisal_output(asset: Asset, time_slice: TimeSliceID) -> AppraisalOutpu activity, demand, unmet_demand, + metric_precedence: 0, metric: 4.14, } } diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 542f643d4..e8e85ce01 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -8,6 +8,7 @@ use crate::model::Model; use crate::output::DataWriter; use crate::region::RegionID; use crate::simulation::CommodityPrices; +use crate::simulation::investment::appraisal::filter_for_minimum_precedence; use crate::time_slice::{TimeSliceID, TimeSliceInfo}; use crate::units::{Capacity, Dimensionless, Flow, FlowPerCapacity}; use anyhow::{Context, Result, bail, ensure}; @@ -708,6 +709,8 @@ fn select_best_assets( outputs_for_opts.push(output); } + outputs_for_opts = filter_for_minimum_precedence(outputs_for_opts); + // Save appraisal results writer.write_appraisal_debug_info( year, diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 3321261cb..6987a7479 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -30,6 +30,10 @@ pub struct AppraisalOutput { pub activity: IndexMap, /// The hypothetical unmet demand following investment in this asset pub unmet_demand: DemandMap, + /// Where there is more than one possible metric for comparing appraisals, this integer + /// indicates the precedence of the metric (lower values have higher precedence). + /// Only metrics with the same precedence should be compared. + pub metric_precedence: u8, /// The comparison metric to compare investment decisions (lower is better) pub metric: f64, /// Capacity and activity coefficients used in the appraisal @@ -112,6 +116,14 @@ pub fn classify_appraisal_comparison_method( } } +/// Filter mixed-precedence appraisal outputs to only those with the minimum metric precedence +pub fn filter_for_minimum_precedence(mut outputs: Vec) -> Vec { + if let Some(min_precedence) = outputs.iter().map(|o| o.metric_precedence).min() { + outputs.retain(|o| o.metric_precedence == min_precedence); + } + outputs +} + /// 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 @@ -151,6 +163,7 @@ fn calculate_lcox( capacity: results.capacity, activity: results.activity, unmet_demand: results.unmet_demand, + metric_precedence: 0, metric: cost_index.value(), coefficients: coefficients.clone(), demand: demand.clone(), @@ -187,14 +200,23 @@ fn calculate_npv( activity_surpluses, ); + // calculate metric and precedence depending on asset parameters + // note that metric will be minimised so if larger is better, we negate the value + let (metric_precedence, metric) = match annual_fixed_cost.value() { + // If AFC is zero, use total surplus as the metric (strictly better than nonzero AFC) + 0.0 => (0, -profitability_index.total_annualised_surplus.value()), + // If AFC is non-zero, use profitability index as the metric + _ => (1, -profitability_index.value().value()), + }; + // Return appraisal output - // Higher profitability index is better, so we make it negative for comparison Ok(AppraisalOutput { asset: asset.clone(), capacity: results.capacity, activity: results.activity, unmet_demand: results.unmet_demand, - metric: -profitability_index.value().value(), + metric_precedence, + metric, coefficients: coefficients.clone(), demand: demand.clone(), }) @@ -216,3 +238,60 @@ pub fn appraise_investment( }; appraisal_method(model, asset, max_capacity, commodity, coefficients, demand) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::asset::Asset; + use crate::fixture::{asset, time_slice}; + use crate::units::{ + Activity, Capacity, Flow, MoneyPerActivity, MoneyPerCapacity, MoneyPerFlow, + }; + use indexmap::indexmap; + use rstest::rstest; + + /// Create an AppraisalOutput with customisable metric precedence for testing + fn appraisal_output( + asset: Asset, + time_slice: TimeSliceID, + metric_precedence: u8, + ) -> AppraisalOutput { + let activity_coefficients = indexmap! { time_slice.clone() => MoneyPerActivity(0.5) }; + let activity = indexmap! { time_slice.clone() => Activity(10.0) }; + let demand = indexmap! { time_slice.clone() => Flow(100.0) }; + let unmet_demand = indexmap! { time_slice.clone() => Flow(5.0) }; + + AppraisalOutput { + asset: AssetRef::from(asset), + capacity: Capacity(42.0), + coefficients: ObjectiveCoefficients { + capacity_coefficient: MoneyPerCapacity(3.14), + activity_coefficients, + unmet_demand_coefficient: MoneyPerFlow(10000.0), + }, + activity, + demand, + unmet_demand, + metric_precedence, + metric: 4.14, + } + } + + #[rstest] + fn test_filter_for_minimum_precedence(asset: Asset, time_slice: TimeSliceID) { + let outputs = vec![ + appraisal_output(asset.clone(), time_slice.clone(), 1), + appraisal_output(asset.clone(), time_slice.clone(), 0), + appraisal_output(asset.clone(), time_slice.clone(), 2), + appraisal_output(asset.clone(), time_slice.clone(), 0), + appraisal_output(asset.clone(), time_slice.clone(), 1), + appraisal_output(asset, time_slice, 1), + ]; + + let filtered = filter_for_minimum_precedence(outputs); + + assert_eq!(filtered.len(), 2); + assert_eq!(filtered[0].metric_precedence, 0); + assert_eq!(filtered[1].metric_precedence, 0); + } +} From d8dead16f41db890d33ac659eb118af9c0bf7b93 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 10 Dec 2025 16:22:17 +0000 Subject: [PATCH 3/7] add comment for clarity --- src/simulation/investment.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index e8e85ce01..d8a5c21be 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -709,6 +709,7 @@ fn select_best_assets( outputs_for_opts.push(output); } + // discard any appraisals with non-minimal metric precedence outputs_for_opts = filter_for_minimum_precedence(outputs_for_opts); // Save appraisal results From 8baf8284cc6042d2ec0e3a999158fe71e58b58b8 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 10 Dec 2025 16:28:59 +0000 Subject: [PATCH 4/7] add bail for negative AFC --- src/simulation/investment/appraisal.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 6987a7479..215b020fa 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -7,7 +7,7 @@ use crate::finance::{lcox, profitability_index}; use crate::model::Model; use crate::time_slice::TimeSliceID; use crate::units::{Activity, Capacity}; -use anyhow::Result; +use anyhow::{Result, bail}; use costs::annual_fixed_cost; use indexmap::IndexMap; use std::cmp::Ordering; @@ -192,6 +192,9 @@ fn calculate_npv( // Calculate profitability index for the hypothetical investment let annual_fixed_cost = annual_fixed_cost(asset); + if annual_fixed_cost.value() < 0.0 { + bail!("The current NPV calculation does not support negative annual fixed costs"); + } let activity_surpluses = &coefficients.activity_coefficients; let profitability_index = profitability_index( results.capacity, From bdcf224ca22c789da47b2a94ab117ed51e0b09b7 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 10 Dec 2025 16:45:13 +0000 Subject: [PATCH 5/7] clearer comment --- src/simulation/investment/appraisal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 215b020fa..3c9dd78be 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -253,7 +253,7 @@ mod tests { use indexmap::indexmap; use rstest::rstest; - /// Create an AppraisalOutput with customisable metric precedence for testing + /// Create an AppraisalOutput with customisable metric precedence fn appraisal_output( asset: Asset, time_slice: TimeSliceID, From 5f5b59baa23211e15c61a9b1d2f5733c02ec86c8 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Tue, 6 Jan 2026 10:16:40 +0000 Subject: [PATCH 6/7] implement traits approach to comparing metrics --- src/finance.rs | 1 + src/fixture.rs | 4 +- src/output.rs | 2 +- src/simulation/investment.rs | 4 - src/simulation/investment/appraisal.rs | 255 ++++++++++++++----------- 5 files changed, 151 insertions(+), 115 deletions(-) diff --git a/src/finance.rs b/src/finance.rs index d0731e0ae..ee8839252 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -30,6 +30,7 @@ pub fn annual_capital_cost( /// Represents the profitability index of an investment /// in terms of it's annualised components. +#[derive(Debug, Clone, Copy)] pub struct ProfitabilityIndex { /// the total annualised surplus of an asset pub total_annualised_surplus: Money, diff --git a/src/fixture.rs b/src/fixture.rs index 090532cb1..d966d20d1 100644 --- a/src/fixture.rs +++ b/src/fixture.rs @@ -14,6 +14,7 @@ use crate::process::{ ProcessInvestmentConstraintsMap, ProcessMap, ProcessParameter, ProcessParameterMap, }; use crate::region::RegionID; +use crate::simulation::investment::appraisal::LCOXMetric; use crate::simulation::investment::appraisal::{ AppraisalOutput, coefficients::ObjectiveCoefficients, }; @@ -360,8 +361,7 @@ pub fn appraisal_output(asset: Asset, time_slice: TimeSliceID) -> AppraisalOutpu activity, demand, unmet_demand, - metric_precedence: 0, - metric: 4.14, + metric: Box::new(LCOXMetric::new(4.14)), } } diff --git a/src/output.rs b/src/output.rs index 7c1f67b1e..6e63d57f1 100644 --- a/src/output.rs +++ b/src/output.rs @@ -453,7 +453,7 @@ impl DebugDataWriter { region_id: result.asset.region_id().clone(), capacity: result.capacity, capacity_coefficient: result.coefficients.capacity_coefficient, - metric: result.metric, + metric: result.metric.value(), }; self.appraisal_results_writer.serialize(row)?; } diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 540776ce3..2900ac899 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -8,7 +8,6 @@ use crate::model::Model; use crate::output::DataWriter; use crate::region::RegionID; use crate::simulation::CommodityPrices; -use crate::simulation::investment::appraisal::filter_for_minimum_precedence; use crate::time_slice::{TimeSliceID, TimeSliceInfo}; use crate::units::{Capacity, Dimensionless, Flow, FlowPerCapacity}; use anyhow::{Context, Result, bail, ensure}; @@ -719,9 +718,6 @@ fn select_best_assets( outputs_for_opts.push(output); } - // discard any appraisals with non-minimal metric precedence - outputs_for_opts = filter_for_minimum_precedence(outputs_for_opts); - // Save appraisal results writer.write_appraisal_debug_info( year, diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index ce01714a9..68f8dfb6b 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -3,13 +3,14 @@ use super::DemandMap; use crate::agent::ObjectiveType; use crate::asset::AssetRef; use crate::commodity::Commodity; -use crate::finance::{lcox, profitability_index}; +use crate::finance::{ProfitabilityIndex, lcox, profitability_index}; use crate::model::Model; use crate::time_slice::TimeSliceID; use crate::units::{Activity, Capacity}; -use anyhow::{Result, bail}; +use anyhow::Result; use costs::annual_fixed_cost; use indexmap::IndexMap; +use std::any::Any; use std::cmp::Ordering; pub mod coefficients; @@ -30,12 +31,8 @@ pub struct AppraisalOutput { pub activity: IndexMap, /// The hypothetical unmet demand following investment in this asset pub unmet_demand: DemandMap, - /// Where there is more than one possible metric for comparing appraisals, this integer - /// indicates the precedence of the metric (lower values have higher precedence). - /// Only metrics with the same precedence should be compared. - pub metric_precedence: u8, /// The comparison metric to compare investment decisions (lower is better) - pub metric: f64, + pub metric: Box, /// Capacity and activity coefficients used in the appraisal pub coefficients: ObjectiveCoefficients, /// Demand profile used in the appraisal @@ -52,31 +49,149 @@ impl AppraisalOutput { /// depending on the user's platform (e.g. macOS ARM vs. Windows). We want to avoid this, if /// possible, which is why we use a more approximate comparison. pub fn compare_metric(&self, other: &Self) -> Ordering { - assert!( - !(self.metric.is_nan() || other.metric.is_nan()), - "Appraisal metric cannot be NaN" - ); + self.metric.compare(other.metric.as_ref()) + } +} + +/// Trait for appraisal metrics that can be compared. +/// +/// Implementers define how their values should be compared to determine +/// which investment option is preferable through the `compare` method. +pub trait MetricTrait: Any + Send + Sync { + /// Returns the numeric value of this metric. + fn value(&self) -> f64; + + /// Compares this metric with another of the same type. + /// + /// Returns `Ordering::Less` if `self` is better than `other`, + /// `Ordering::Greater` if `other` is better, or `Ordering::Equal` + /// if they are approximately equal. + /// + /// # Panics + /// + /// Panics if `other` is not the same concrete type as `self`. + fn compare(&self, other: &dyn MetricTrait) -> Ordering; + + /// Helper for downcasting to enable type-safe comparison. + fn as_any(&self) -> &dyn Any; +} - if approx_eq!(f64, self.metric, other.metric) { +/// Levelised Cost of X (LCOX) metric. +/// +/// Represents the average cost per unit of output. Lower values indicate +/// more cost-effective investments. +#[derive(Debug, Clone)] +pub struct LCOXMetric { + /// The calculated cost value for this LCOX metric + pub cost: f64, +} + +impl LCOXMetric { + /// Creates a new `LCOXMetric` with the given cost. + pub fn new(cost: f64) -> Self { + // assert!(!cost.is_nan(), "LCOX metric cannot be NaN"); + Self { cost } + } +} + +impl MetricTrait for LCOXMetric { + fn value(&self) -> f64 { + self.cost + } + + fn compare(&self, other: &dyn MetricTrait) -> Ordering { + let other = other + .as_any() + .downcast_ref::() + .expect("Cannot compare metrics of different types"); + + if approx_eq!(f64, self.cost, other.cost) { Ordering::Equal } else { - self.metric.partial_cmp(&other.metric).unwrap() + // Lower cost is better + self.cost.partial_cmp(&other.cost).unwrap() } } + + fn as_any(&self) -> &dyn Any { + self + } } -/// Filter mixed-precedence appraisal outputs to only those with the minimum metric precedence -pub fn filter_for_minimum_precedence(mut outputs: Vec) -> Vec { - if let Some(min_precedence) = outputs.iter().map(|o| o.metric_precedence).min() { - outputs.retain(|o| o.metric_precedence == min_precedence); +/// Net Present Value (NPV) metric +#[derive(Debug, Clone)] +pub struct NPVMetric { + /// Profitability index data for this NPV metric + pub profitability_index: ProfitabilityIndex, +} + +impl NPVMetric { + /// Creates a new `NPVMetric` with the given profitability index. + pub fn new(profitability_index: ProfitabilityIndex) -> Self { + Self { + profitability_index, + } + } + + /// Returns true if this metric represents a zero fixed cost case. + fn is_zero_fixed_cost(&self) -> bool { + self.profitability_index.annualised_fixed_cost.value() == 0.0 + } +} + +impl MetricTrait for NPVMetric { + fn value(&self) -> f64 { + if self.is_zero_fixed_cost() { + self.profitability_index.total_annualised_surplus.value() + } else { + self.profitability_index.value().value() + } + } + + /// Higher profitability index values indicate more profitable investments. + /// When annual fixed cost is zero, the profitability index is infinite and + /// total surplus is used for comparison instead. + fn compare(&self, other: &dyn MetricTrait) -> Ordering { + let other = other + .as_any() + .downcast_ref::() + .expect("Cannot compare metrics of different types"); + + // Handle comparison based on fixed cost status + match (self.is_zero_fixed_cost(), other.is_zero_fixed_cost()) { + // Both have zero fixed cost: compare total surplus (higher is better) + (true, true) => { + let self_surplus = self.profitability_index.total_annualised_surplus.value(); + let other_surplus = other.profitability_index.total_annualised_surplus.value(); + + if approx_eq!(f64, self_surplus, other_surplus) { + Ordering::Equal + } else { + other_surplus.partial_cmp(&self_surplus).unwrap() + } + } + // Both have non-zero fixed cost: compare profitability index (higher is better) + (false, false) => { + let self_pi = self.profitability_index.value().value(); + let other_pi = other.profitability_index.value().value(); + + if approx_eq!(f64, self_pi, other_pi) { + Ordering::Equal + } else { + other_pi.partial_cmp(&self_pi).unwrap() + } + } + // Zero fixed cost is always better than non-zero fixed cost + (true, false) => Ordering::Less, + (false, true) => Ordering::Greater, + } + } + + fn as_any(&self) -> &dyn Any { + self } - outputs } -/// 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 -/// include other flows, we use the term LCOX. fn calculate_lcox( model: &Model, asset: &AssetRef, @@ -85,7 +200,6 @@ fn calculate_lcox( coefficients: &ObjectiveCoefficients, demand: &DemandMap, ) -> Result { - // Perform optimisation to calculate capacity, activity and unmet demand let results = perform_optimisation( asset, max_capacity, @@ -96,30 +210,24 @@ fn calculate_lcox( highs::Sense::Minimise, )?; - // Calculate LCOX for the hypothetical investment - let annual_fixed_cost = coefficients.capacity_coefficient; - let activity_costs = &coefficients.activity_coefficients; let cost_index = lcox( results.capacity, - annual_fixed_cost, + coefficients.capacity_coefficient, &results.activity, - activity_costs, + &coefficients.activity_coefficients, ); - // Return appraisal output Ok(AppraisalOutput { asset: asset.clone(), capacity: results.capacity, activity: results.activity, unmet_demand: results.unmet_demand, - metric_precedence: 0, - metric: cost_index.value(), + metric: Box::new(LCOXMetric::new(cost_index.value())), coefficients: coefficients.clone(), demand: demand.clone(), }) } -/// Calculate NPV for a hypothetical investment in the given asset. fn calculate_npv( model: &Model, asset: &AssetRef, @@ -128,7 +236,6 @@ fn calculate_npv( coefficients: &ObjectiveCoefficients, demand: &DemandMap, ) -> Result { - // Perform optimisation to calculate capacity, activity and unmet demand let results = perform_optimisation( asset, max_capacity, @@ -139,36 +246,25 @@ fn calculate_npv( highs::Sense::Maximise, )?; - // Calculate profitability index for the hypothetical investment let annual_fixed_cost = annual_fixed_cost(asset); - if annual_fixed_cost.value() < 0.0 { - bail!("The current NPV calculation does not support negative annual fixed costs"); - } - let activity_surpluses = &coefficients.activity_coefficients; + assert!( + annual_fixed_cost.value() >= 0.0, + "The current NPV calculation does not support negative annual fixed costs" + ); + let profitability_index = profitability_index( results.capacity, annual_fixed_cost, &results.activity, - activity_surpluses, + &coefficients.activity_coefficients, ); - // calculate metric and precedence depending on asset parameters - // note that metric will be minimised so if larger is better, we negate the value - let (metric_precedence, metric) = match annual_fixed_cost.value() { - // If AFC is zero, use total surplus as the metric (strictly better than nonzero AFC) - 0.0 => (0, -profitability_index.total_annualised_surplus.value()), - // If AFC is non-zero, use profitability index as the metric - _ => (1, -profitability_index.value().value()), - }; - - // Return appraisal output Ok(AppraisalOutput { asset: asset.clone(), capacity: results.capacity, activity: results.activity, unmet_demand: results.unmet_demand, - metric_precedence, - metric, + metric: Box::new(NPVMetric::new(profitability_index)), coefficients: coefficients.clone(), demand: demand.clone(), }) @@ -190,60 +286,3 @@ pub fn appraise_investment( }; appraisal_method(model, asset, max_capacity, commodity, coefficients, demand) } - -#[cfg(test)] -mod tests { - use super::*; - use crate::asset::Asset; - use crate::fixture::{asset, time_slice}; - use crate::units::{ - Activity, Capacity, Flow, MoneyPerActivity, MoneyPerCapacity, MoneyPerFlow, - }; - use indexmap::indexmap; - use rstest::rstest; - - /// Create an AppraisalOutput with customisable metric precedence - fn appraisal_output( - asset: Asset, - time_slice: TimeSliceID, - metric_precedence: u8, - ) -> AppraisalOutput { - let activity_coefficients = indexmap! { time_slice.clone() => MoneyPerActivity(0.5) }; - let activity = indexmap! { time_slice.clone() => Activity(10.0) }; - let demand = indexmap! { time_slice.clone() => Flow(100.0) }; - let unmet_demand = indexmap! { time_slice.clone() => Flow(5.0) }; - - AppraisalOutput { - asset: AssetRef::from(asset), - capacity: Capacity(42.0), - coefficients: ObjectiveCoefficients { - capacity_coefficient: MoneyPerCapacity(3.14), - activity_coefficients, - unmet_demand_coefficient: MoneyPerFlow(10000.0), - }, - activity, - demand, - unmet_demand, - metric_precedence, - metric: 4.14, - } - } - - #[rstest] - fn test_filter_for_minimum_precedence(asset: Asset, time_slice: TimeSliceID) { - let outputs = vec![ - appraisal_output(asset.clone(), time_slice.clone(), 1), - appraisal_output(asset.clone(), time_slice.clone(), 0), - appraisal_output(asset.clone(), time_slice.clone(), 2), - appraisal_output(asset.clone(), time_slice.clone(), 0), - appraisal_output(asset.clone(), time_slice.clone(), 1), - appraisal_output(asset, time_slice, 1), - ]; - - let filtered = filter_for_minimum_precedence(outputs); - - assert_eq!(filtered.len(), 2); - assert_eq!(filtered[0].metric_precedence, 0); - assert_eq!(filtered[1].metric_precedence, 0); - } -} From d411957f24c2ffa76b1f8b745bb83671125a9290 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Tue, 6 Jan 2026 12:41:44 +0000 Subject: [PATCH 7/7] move nan assertion to compare_metric --- src/simulation/investment/appraisal.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 68f8dfb6b..f376d58ae 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -49,6 +49,10 @@ impl AppraisalOutput { /// depending on the user's platform (e.g. macOS ARM vs. Windows). We want to avoid this, if /// possible, which is why we use a more approximate comparison. pub fn compare_metric(&self, other: &Self) -> Ordering { + assert!( + !(self.metric.value().is_nan() || other.metric.value().is_nan()), + "Appraisal metric cannot be NaN" + ); self.metric.compare(other.metric.as_ref()) } } @@ -89,7 +93,6 @@ pub struct LCOXMetric { impl LCOXMetric { /// Creates a new `LCOXMetric` with the given cost. pub fn new(cost: f64) -> Self { - // assert!(!cost.is_nan(), "LCOX metric cannot be NaN"); Self { cost } } }