From 701ab71b7555eda38624fbeb384a806d5b515561 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Mon, 8 Dec 2025 17:11:42 +0000 Subject: [PATCH 01/17] 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 02/17] 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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 07/17] 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 } } } From 5f669916b0c34c906d5a7e680b068f290589a920 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Thu, 8 Jan 2026 14:24:45 +0000 Subject: [PATCH 08/17] improve adherence to units system --- src/fixture.rs | 2 +- src/simulation/investment/appraisal.rs | 28 +++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/fixture.rs b/src/fixture.rs index dce1db96b..dc01dda35 100644 --- a/src/fixture.rs +++ b/src/fixture.rs @@ -361,7 +361,7 @@ pub fn appraisal_output(asset: Asset, time_slice: TimeSliceID) -> AppraisalOutpu activity, demand, unmet_demand, - metric: Box::new(LCOXMetric::new(4.14)), + metric: Box::new(LCOXMetric::new(MoneyPerActivity(4.14))), } } diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index f376d58ae..592814952 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -6,7 +6,7 @@ use crate::commodity::Commodity; use crate::finance::{ProfitabilityIndex, lcox, profitability_index}; use crate::model::Model; use crate::time_slice::TimeSliceID; -use crate::units::{Activity, Capacity}; +use crate::units::{Activity, Capacity, Dimensionless, Money, MoneyPerActivity, MoneyPerCapacity}; use anyhow::Result; use costs::annual_fixed_cost; use indexmap::IndexMap; @@ -87,19 +87,19 @@ pub trait MetricTrait: Any + Send + Sync { #[derive(Debug, Clone)] pub struct LCOXMetric { /// The calculated cost value for this LCOX metric - pub cost: f64, + pub cost: MoneyPerActivity, } impl LCOXMetric { /// Creates a new `LCOXMetric` with the given cost. - pub fn new(cost: f64) -> Self { + pub fn new(cost: MoneyPerActivity) -> Self { Self { cost } } } impl MetricTrait for LCOXMetric { fn value(&self) -> f64 { - self.cost + self.cost.value() } fn compare(&self, other: &dyn MetricTrait) -> Ordering { @@ -108,7 +108,7 @@ impl MetricTrait for LCOXMetric { .downcast_ref::() .expect("Cannot compare metrics of different types"); - if approx_eq!(f64, self.cost, other.cost) { + if approx_eq!(MoneyPerActivity, self.cost, other.cost) { Ordering::Equal } else { // Lower cost is better @@ -138,7 +138,7 @@ impl NPVMetric { /// 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 + self.profitability_index.annualised_fixed_cost == Money(0.0) } } @@ -164,10 +164,10 @@ impl MetricTrait for NPVMetric { 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(); + let self_surplus = self.profitability_index.total_annualised_surplus; + let other_surplus = other.profitability_index.total_annualised_surplus; - if approx_eq!(f64, self_surplus, other_surplus) { + if approx_eq!(Money, self_surplus, other_surplus) { Ordering::Equal } else { other_surplus.partial_cmp(&self_surplus).unwrap() @@ -175,10 +175,10 @@ impl MetricTrait for NPVMetric { } // 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(); + let self_pi = self.profitability_index.value(); + let other_pi = other.profitability_index.value(); - if approx_eq!(f64, self_pi, other_pi) { + if approx_eq!(Dimensionless, self_pi, other_pi) { Ordering::Equal } else { other_pi.partial_cmp(&self_pi).unwrap() @@ -225,7 +225,7 @@ fn calculate_lcox( capacity: results.capacity, activity: results.activity, unmet_demand: results.unmet_demand, - metric: Box::new(LCOXMetric::new(cost_index.value())), + metric: Box::new(LCOXMetric::new(cost_index)), coefficients: coefficients.clone(), demand: demand.clone(), }) @@ -251,7 +251,7 @@ fn calculate_npv( let annual_fixed_cost = annual_fixed_cost(asset); assert!( - annual_fixed_cost.value() >= 0.0, + annual_fixed_cost >= MoneyPerCapacity(0.0), "The current NPV calculation does not support negative annual fixed costs" ); From 9dc102841407fe2865d5ec374ed99daf27758b6a Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Thu, 8 Jan 2026 14:41:47 +0000 Subject: [PATCH 09/17] add assertion against calculating an infinite profitability index --- src/finance.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/finance.rs b/src/finance.rs index 340bfc322..6e09758d0 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -41,6 +41,10 @@ pub struct ProfitabilityIndex { impl ProfitabilityIndex { /// Calculates the value of the profitability index. pub fn value(&self) -> Dimensionless { + assert!( + self.annualised_fixed_cost != Money(0.0), + "Annualised fixed cost cannot be zero when calculating profitability index." + ); self.total_annualised_surplus / self.annualised_fixed_cost } } From 5e8a65e9348a7cceb7e20e397f60b909fcab1875 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Thu, 8 Jan 2026 15:45:20 +0000 Subject: [PATCH 10/17] correct comments tying update test assertion --- src/finance.rs | 24 +++++++++++++++--------- src/simulation/investment/appraisal.rs | 6 +++--- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/finance.rs b/src/finance.rs index 6e09758d0..8b4c5952a 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -29,12 +29,12 @@ pub fn annual_capital_cost( } /// Represents the profitability index of an investment -/// in terms of it's annualised components. +/// in terms of its annualised components. #[derive(Debug, Clone, Copy)] pub struct ProfitabilityIndex { - /// the total annualised surplus of an asset + /// The total annualised surplus of an asset pub total_annualised_surplus: Money, - /// the total annualised fixed cost of an asset + /// The total annualised fixed cost of an asset pub annualised_fixed_cost: Money, } @@ -150,12 +150,6 @@ mod tests { vec![("q1", "peak", 40.0)], 0.04 // Expected PI: (5*40) / (50*100) = 200/5000 = 0.04 )] - #[case( - 0.0, 100.0, - vec![("winter", "day", 10.0)], - vec![("winter", "day", 50.0)], - f64::INFINITY // Zero capacity case - )] fn profitability_index_works( #[case] capacity: f64, #[case] annual_fixed_cost: f64, @@ -211,6 +205,18 @@ mod tests { assert_eq!(result.value(), Dimensionless(0.0)); } + #[test] + #[should_panic(expected = "Annualised fixed cost cannot be zero")] + fn profitability_index_panics_on_zero_cost() { + let result = profitability_index( + Capacity(0.0), + MoneyPerCapacity(100.0), + &indexmap! {}, + &indexmap! {}, + ); + result.value(); + } + #[rstest] #[case( 100.0, 50.0, diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index fc8cff620..d6286303b 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -31,7 +31,7 @@ pub struct AppraisalOutput { pub activity: IndexMap, /// The hypothetical unmet demand following investment in this asset pub unmet_demand: DemandMap, - /// The comparison metric to compare investment decisions (lower is better) + /// The comparison metric to compare investment decisions pub metric: Box, /// Capacity and activity coefficients used in the appraisal pub coefficients: ObjectiveCoefficients, @@ -246,7 +246,7 @@ fn calculate_lcox( /// /// An `AppraisalOutput` containing the hypothetical capacity, activity profile and unmet demand. /// The returned `metric` is the negative of the profitability index so that, like LCOX, -/// lower metric values indicate a more desirable investment (i.e. higher NPV). +/// higher metric values indicate a more desirable investment (i.e. higher NPV). fn calculate_npv( model: &Model, asset: &AssetRef, @@ -294,7 +294,7 @@ fn calculate_npv( /// # Returns /// /// The `AppraisalOutput` produced by the selected appraisal method. The `metric` field is -/// comparable across methods where lower values indicate a better investment. +/// comparable across methods. pub fn appraise_investment( model: &Model, asset: &AssetRef, From 29e38a39cff286cd55c8b781596d1876d428b6c0 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Thu, 8 Jan 2026 15:57:40 +0000 Subject: [PATCH 11/17] correct comments --- src/simulation/investment/appraisal.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index d6286303b..b18beb981 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -245,8 +245,6 @@ fn calculate_lcox( /// # Returns /// /// An `AppraisalOutput` containing the hypothetical capacity, activity profile and unmet demand. -/// The returned `metric` is the negative of the profitability index so that, like LCOX, -/// higher metric values indicate a more desirable investment (i.e. higher NPV). fn calculate_npv( model: &Model, asset: &AssetRef, @@ -294,7 +292,7 @@ fn calculate_npv( /// # Returns /// /// The `AppraisalOutput` produced by the selected appraisal method. The `metric` field is -/// comparable across methods. +/// comparable with other appraisals of the same type (npv/lcox). pub fn appraise_investment( model: &Model, asset: &AssetRef, From 9a7f6cde631e66e1ff67cc7190b02ce261b52c17 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Mon, 12 Jan 2026 10:54:05 +0000 Subject: [PATCH 12/17] simplify npv metric struct --- src/simulation/investment/appraisal.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index b18beb981..705f567f9 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -123,31 +123,26 @@ impl MetricTrait for LCOXMetric { /// Net Present Value (NPV) metric #[derive(Debug, Clone)] -pub struct NPVMetric { - /// Profitability index data for this NPV metric - pub profitability_index: ProfitabilityIndex, -} +pub struct NPVMetric(ProfitabilityIndex); impl NPVMetric { /// Creates a new `NPVMetric` with the given profitability index. pub fn new(profitability_index: ProfitabilityIndex) -> Self { - Self { - profitability_index, - } + 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 == Money(0.0) + self.0.annualised_fixed_cost == Money(0.0) } } impl MetricTrait for NPVMetric { fn value(&self) -> f64 { if self.is_zero_fixed_cost() { - self.profitability_index.total_annualised_surplus.value() + self.0.total_annualised_surplus.value() } else { - self.profitability_index.value().value() + self.0.value().value() } } @@ -164,8 +159,8 @@ impl MetricTrait for NPVMetric { 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; - let other_surplus = other.profitability_index.total_annualised_surplus; + let self_surplus = self.0.total_annualised_surplus; + let other_surplus = other.0.total_annualised_surplus; if approx_eq!(Money, self_surplus, other_surplus) { Ordering::Equal @@ -175,8 +170,8 @@ impl MetricTrait for NPVMetric { } // Both have non-zero fixed cost: compare profitability index (higher is better) (false, false) => { - let self_pi = self.profitability_index.value(); - let other_pi = other.profitability_index.value(); + let self_pi = self.0.value(); + let other_pi = other.0.value(); if approx_eq!(Dimensionless, self_pi, other_pi) { Ordering::Equal From e5861b3eec610bfb57cfbd3772ceebe9c0e2ca6c Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Tue, 13 Jan 2026 14:57:27 +0000 Subject: [PATCH 13/17] Add erased serialize dependency Add serialize implementations to metric types --- Cargo.lock | 18 +++++++++++++++++ Cargo.toml | 1 + src/finance.rs | 3 ++- src/simulation/investment/appraisal.rs | 28 ++++++++++++++++++-------- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a00e9e9d6..2edc6a584 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -433,6 +433,17 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f" +[[package]] +name = "erased-serde" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "89e8918065695684b2b0702da20382d5ae6065cf3327bc2d6436bd49a71ce9f3" +dependencies = [ + "serde", + "serde_core", + "typeid", +] + [[package]] name = "errno" version = "0.3.14" @@ -959,6 +970,7 @@ dependencies = [ "dirs", "documented", "edit", + "erased-serde", "fern", "float-cmp", "highs", @@ -1569,6 +1581,12 @@ version = "1.0.6+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab16f14aed21ee8bfd8ec22513f7287cd4a91aa92e44edfe2c17ddd004e92607" +[[package]] +name = "typeid" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c" + [[package]] name = "unicase" version = "2.8.1" diff --git a/Cargo.toml b/Cargo.toml index 79d15f479..eaa30185a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ strum = {version = "0.27.2", features = ["derive"]} documented = "0.9.2" dirs = "6.0.0" edit = "0.1.5" +erased-serde = "0.4.9" [dev-dependencies] map-macro = "0.3.0" diff --git a/src/finance.rs b/src/finance.rs index 8b4c5952a..495baa54b 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -2,6 +2,7 @@ use crate::time_slice::TimeSliceID; use crate::units::{Activity, Capacity, Dimensionless, Money, MoneyPerActivity, MoneyPerCapacity}; use indexmap::IndexMap; +use serde::Serialize; /// Calculates the capital recovery factor (CRF) for a given lifetime and discount rate. /// @@ -30,7 +31,7 @@ pub fn annual_capital_cost( /// Represents the profitability index of an investment /// in terms of its annualised components. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Serialize)] pub struct ProfitabilityIndex { /// The total annualised surplus of an asset pub total_annualised_surplus: Money, diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 705f567f9..66222dd04 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -9,7 +9,9 @@ use crate::time_slice::TimeSliceID; use crate::units::{Activity, Capacity, Dimensionless, Money, MoneyPerActivity, MoneyPerCapacity}; use anyhow::Result; use costs::annual_fixed_cost; +use erased_serde::Serialize as ErasedSerialize; use indexmap::IndexMap; +use serde::Serialize; use std::any::Any; use std::cmp::Ordering; @@ -57,11 +59,15 @@ impl AppraisalOutput { } } +/// Supertrait for appraisal metrics that can be serialised and compared. +pub trait MetricTrait: ComparableMetric + ErasedSerialize {} +erased_serde::serialize_trait_object!(MetricTrait); + /// 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 { +pub trait ComparableMetric: Any + Send + Sync { /// Returns the numeric value of this metric. fn value(&self) -> f64; @@ -74,7 +80,7 @@ pub trait MetricTrait: Any + Send + Sync { /// # Panics /// /// Panics if `other` is not the same concrete type as `self`. - fn compare(&self, other: &dyn MetricTrait) -> Ordering; + fn compare(&self, other: &dyn ComparableMetric) -> Ordering; /// Helper for downcasting to enable type-safe comparison. fn as_any(&self) -> &dyn Any; @@ -84,7 +90,7 @@ pub trait MetricTrait: Any + Send + Sync { /// /// Represents the average cost per unit of output. Lower values indicate /// more cost-effective investments. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize)] pub struct LCOXMetric { /// The calculated cost value for this LCOX metric pub cost: MoneyPerActivity, @@ -97,12 +103,12 @@ impl LCOXMetric { } } -impl MetricTrait for LCOXMetric { +impl ComparableMetric for LCOXMetric { fn value(&self) -> f64 { self.cost.value() } - fn compare(&self, other: &dyn MetricTrait) -> Ordering { + fn compare(&self, other: &dyn ComparableMetric) -> Ordering { let other = other .as_any() .downcast_ref::() @@ -121,8 +127,11 @@ impl MetricTrait for LCOXMetric { } } +/// `LCOXMetric` implements the `MetricTrait` supertrait. +impl MetricTrait for LCOXMetric {} + /// Net Present Value (NPV) metric -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize)] pub struct NPVMetric(ProfitabilityIndex); impl NPVMetric { @@ -137,7 +146,7 @@ impl NPVMetric { } } -impl MetricTrait for NPVMetric { +impl ComparableMetric for NPVMetric { fn value(&self) -> f64 { if self.is_zero_fixed_cost() { self.0.total_annualised_surplus.value() @@ -149,7 +158,7 @@ impl MetricTrait for NPVMetric { /// 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 { + fn compare(&self, other: &dyn ComparableMetric) -> Ordering { let other = other .as_any() .downcast_ref::() @@ -190,6 +199,9 @@ impl MetricTrait for NPVMetric { } } +/// `NPVMetric` implements the `MetricTrait` supertrait. +impl MetricTrait for NPVMetric {} + /// 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 534da283f6328b9e2891998ff1a3b60c30e73ccd Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Tue, 13 Jan 2026 17:22:36 +0000 Subject: [PATCH 14/17] add serde json and serialisation for metrics --- Cargo.lock | 20 +++++++++++ Cargo.toml | 1 + src/input/process/flow.rs | 2 +- src/output.rs | 7 ++-- src/simulation/investment/appraisal.rs | 47 ++++++++++++++++---------- 5 files changed, 55 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2edc6a584..2c9bb11ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -984,6 +984,7 @@ dependencies = [ "platform-info", "rstest", "serde", + "serde_json", "serde_string_enum", "strum", "tempfile", @@ -1373,6 +1374,19 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_json" +version = "1.0.149" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86" +dependencies = [ + "itoa", + "memchr", + "serde", + "serde_core", + "zmij", +] + [[package]] name = "serde_spanned" version = "1.0.4" @@ -2139,3 +2153,9 @@ dependencies = [ "quote", "syn", ] + +[[package]] +name = "zmij" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd8f3f50b848df28f887acb68e41201b5aea6bc8a8dacc00fb40635ff9a72fea" diff --git a/Cargo.toml b/Cargo.toml index eaa30185a..2818f07dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ documented = "0.9.2" dirs = "6.0.0" edit = "0.1.5" erased-serde = "0.4.9" +serde_json = "1.0.149" [dev-dependencies] map-macro = "0.3.0" diff --git a/src/input/process/flow.rs b/src/input/process/flow.rs index 9e3228702..4c54d5d35 100644 --- a/src/input/process/flow.rs +++ b/src/input/process/flow.rs @@ -313,7 +313,7 @@ fn validate_secondary_flows( for ((commodity_id, region_id), value) in &flows { ensure!( number_of_years[&(commodity_id.clone(), region_id.clone())] - == required_years.len().try_into().unwrap(), + == u32::try_from(required_years.len()).unwrap(), "Flow of commodity {commodity_id} in region {region_id} for process {process_id} \ does not cover all milestone years within the process range of activity." ); diff --git a/src/output.rs b/src/output.rs index f1968f764..445ae1911 100644 --- a/src/output.rs +++ b/src/output.rs @@ -220,7 +220,7 @@ struct AppraisalResultsRow { region_id: RegionID, capacity: Capacity, capacity_coefficient: MoneyPerCapacity, - metric: f64, + metric: String, } /// Represents the appraisal results in a row of the appraisal results CSV file @@ -453,7 +453,8 @@ impl DebugDataWriter { region_id: result.asset.region_id().clone(), capacity: result.capacity, capacity_coefficient: result.coefficients.capacity_coefficient, - metric: result.metric.value(), + metric: serde_json::to_string(&result.metric) + .context("Failed to serialise metric")?, }; self.appraisal_results_writer.serialize(row)?; } @@ -986,7 +987,7 @@ mod tests { region_id: asset.region_id().clone(), capacity: Capacity(42.0), capacity_coefficient: MoneyPerCapacity(2.14), - metric: 4.14, + metric: "{\"cost\":4.14}".to_string(), }; let records: Vec = csv::Reader::from_path(dir.path().join(APPRAISAL_RESULTS_FILE_NAME)) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 66222dd04..64a10f627 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -6,7 +6,7 @@ use crate::commodity::Commodity; use crate::finance::{ProfitabilityIndex, lcox, profitability_index}; use crate::model::Model; use crate::time_slice::TimeSliceID; -use crate::units::{Activity, Capacity, Dimensionless, Money, MoneyPerActivity, MoneyPerCapacity}; +use crate::units::{Activity, Capacity, Money, MoneyPerActivity, MoneyPerCapacity}; use anyhow::Result; use costs::annual_fixed_cost; use erased_serde::Serialize as ErasedSerialize; @@ -21,8 +21,32 @@ mod costs; mod optimisation; use coefficients::ObjectiveCoefficients; use float_cmp::approx_eq; +use float_cmp::{ApproxEq, F64Margin}; use optimisation::perform_optimisation; +/// Compares two values with approximate equality checking. +/// +/// Returns `Ordering::Equal` if the values are approximately equal +/// according to the default floating-point margin, otherwise returns +/// their relative ordering based on `a.partial_cmp(&b)`. +/// +/// This is useful when comparing floating-point-based types where exact +/// equality may not be appropriate due to numerical precision limitations. +/// +/// # Panics +/// +/// Panics if `partial_cmp` returns `None` (i.e., if either value is NaN). +fn compare_approx(a: T, b: T) -> Ordering +where + T: Copy + PartialOrd + ApproxEq, +{ + if a.approx_eq(b, F64Margin::default()) { + Ordering::Equal + } else { + a.partial_cmp(&b).unwrap() + } +} + /// The output of investment appraisal required to compare potential investment decisions pub struct AppraisalOutput { /// The asset being appraised @@ -114,12 +138,7 @@ impl ComparableMetric for LCOXMetric { .downcast_ref::() .expect("Cannot compare metrics of different types"); - if approx_eq!(MoneyPerActivity, self.cost, other.cost) { - Ordering::Equal - } else { - // Lower cost is better - self.cost.partial_cmp(&other.cost).unwrap() - } + compare_approx(self.cost, other.cost) } fn as_any(&self) -> &dyn Any { @@ -142,7 +161,7 @@ impl NPVMetric { /// Returns true if this metric represents a zero fixed cost case. fn is_zero_fixed_cost(&self) -> bool { - self.0.annualised_fixed_cost == Money(0.0) + approx_eq!(Money, self.0.annualised_fixed_cost, Money(0.0)) } } @@ -171,22 +190,14 @@ impl ComparableMetric for NPVMetric { let self_surplus = self.0.total_annualised_surplus; let other_surplus = other.0.total_annualised_surplus; - if approx_eq!(Money, self_surplus, other_surplus) { - Ordering::Equal - } else { - other_surplus.partial_cmp(&self_surplus).unwrap() - } + compare_approx(other_surplus, self_surplus) } // Both have non-zero fixed cost: compare profitability index (higher is better) (false, false) => { let self_pi = self.0.value(); let other_pi = other.0.value(); - if approx_eq!(Dimensionless, self_pi, other_pi) { - Ordering::Equal - } else { - other_pi.partial_cmp(&self_pi).unwrap() - } + compare_approx(other_pi, self_pi) } // Zero fixed cost is always better than non-zero fixed cost (true, false) => Ordering::Less, From 0e4185bea36acb0d3d8de7e28aa31e210b2bb33a Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 14 Jan 2026 09:22:10 +0000 Subject: [PATCH 15/17] Revert "add serde json and serialisation for metrics" This reverts commit 534da283f6328b9e2891998ff1a3b60c30e73ccd. --- Cargo.lock | 20 ----------- Cargo.toml | 1 - src/input/process/flow.rs | 2 +- src/output.rs | 7 ++-- src/simulation/investment/appraisal.rs | 47 ++++++++++---------------- 5 files changed, 22 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2c9bb11ae..2edc6a584 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -984,7 +984,6 @@ dependencies = [ "platform-info", "rstest", "serde", - "serde_json", "serde_string_enum", "strum", "tempfile", @@ -1374,19 +1373,6 @@ dependencies = [ "syn", ] -[[package]] -name = "serde_json" -version = "1.0.149" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86" -dependencies = [ - "itoa", - "memchr", - "serde", - "serde_core", - "zmij", -] - [[package]] name = "serde_spanned" version = "1.0.4" @@ -2153,9 +2139,3 @@ dependencies = [ "quote", "syn", ] - -[[package]] -name = "zmij" -version = "1.0.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd8f3f50b848df28f887acb68e41201b5aea6bc8a8dacc00fb40635ff9a72fea" diff --git a/Cargo.toml b/Cargo.toml index 2818f07dc..eaa30185a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,6 @@ documented = "0.9.2" dirs = "6.0.0" edit = "0.1.5" erased-serde = "0.4.9" -serde_json = "1.0.149" [dev-dependencies] map-macro = "0.3.0" diff --git a/src/input/process/flow.rs b/src/input/process/flow.rs index 4c54d5d35..9e3228702 100644 --- a/src/input/process/flow.rs +++ b/src/input/process/flow.rs @@ -313,7 +313,7 @@ fn validate_secondary_flows( for ((commodity_id, region_id), value) in &flows { ensure!( number_of_years[&(commodity_id.clone(), region_id.clone())] - == u32::try_from(required_years.len()).unwrap(), + == required_years.len().try_into().unwrap(), "Flow of commodity {commodity_id} in region {region_id} for process {process_id} \ does not cover all milestone years within the process range of activity." ); diff --git a/src/output.rs b/src/output.rs index 445ae1911..f1968f764 100644 --- a/src/output.rs +++ b/src/output.rs @@ -220,7 +220,7 @@ struct AppraisalResultsRow { region_id: RegionID, capacity: Capacity, capacity_coefficient: MoneyPerCapacity, - metric: String, + metric: f64, } /// Represents the appraisal results in a row of the appraisal results CSV file @@ -453,8 +453,7 @@ impl DebugDataWriter { region_id: result.asset.region_id().clone(), capacity: result.capacity, capacity_coefficient: result.coefficients.capacity_coefficient, - metric: serde_json::to_string(&result.metric) - .context("Failed to serialise metric")?, + metric: result.metric.value(), }; self.appraisal_results_writer.serialize(row)?; } @@ -987,7 +986,7 @@ mod tests { region_id: asset.region_id().clone(), capacity: Capacity(42.0), capacity_coefficient: MoneyPerCapacity(2.14), - metric: "{\"cost\":4.14}".to_string(), + metric: 4.14, }; let records: Vec = csv::Reader::from_path(dir.path().join(APPRAISAL_RESULTS_FILE_NAME)) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 64a10f627..66222dd04 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -6,7 +6,7 @@ use crate::commodity::Commodity; use crate::finance::{ProfitabilityIndex, lcox, profitability_index}; use crate::model::Model; use crate::time_slice::TimeSliceID; -use crate::units::{Activity, Capacity, Money, MoneyPerActivity, MoneyPerCapacity}; +use crate::units::{Activity, Capacity, Dimensionless, Money, MoneyPerActivity, MoneyPerCapacity}; use anyhow::Result; use costs::annual_fixed_cost; use erased_serde::Serialize as ErasedSerialize; @@ -21,32 +21,8 @@ mod costs; mod optimisation; use coefficients::ObjectiveCoefficients; use float_cmp::approx_eq; -use float_cmp::{ApproxEq, F64Margin}; use optimisation::perform_optimisation; -/// Compares two values with approximate equality checking. -/// -/// Returns `Ordering::Equal` if the values are approximately equal -/// according to the default floating-point margin, otherwise returns -/// their relative ordering based on `a.partial_cmp(&b)`. -/// -/// This is useful when comparing floating-point-based types where exact -/// equality may not be appropriate due to numerical precision limitations. -/// -/// # Panics -/// -/// Panics if `partial_cmp` returns `None` (i.e., if either value is NaN). -fn compare_approx(a: T, b: T) -> Ordering -where - T: Copy + PartialOrd + ApproxEq, -{ - if a.approx_eq(b, F64Margin::default()) { - Ordering::Equal - } else { - a.partial_cmp(&b).unwrap() - } -} - /// The output of investment appraisal required to compare potential investment decisions pub struct AppraisalOutput { /// The asset being appraised @@ -138,7 +114,12 @@ impl ComparableMetric for LCOXMetric { .downcast_ref::() .expect("Cannot compare metrics of different types"); - compare_approx(self.cost, other.cost) + if approx_eq!(MoneyPerActivity, self.cost, other.cost) { + Ordering::Equal + } else { + // Lower cost is better + self.cost.partial_cmp(&other.cost).unwrap() + } } fn as_any(&self) -> &dyn Any { @@ -161,7 +142,7 @@ impl NPVMetric { /// Returns true if this metric represents a zero fixed cost case. fn is_zero_fixed_cost(&self) -> bool { - approx_eq!(Money, self.0.annualised_fixed_cost, Money(0.0)) + self.0.annualised_fixed_cost == Money(0.0) } } @@ -190,14 +171,22 @@ impl ComparableMetric for NPVMetric { let self_surplus = self.0.total_annualised_surplus; let other_surplus = other.0.total_annualised_surplus; - compare_approx(other_surplus, self_surplus) + if approx_eq!(Money, 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.0.value(); let other_pi = other.0.value(); - compare_approx(other_pi, self_pi) + if approx_eq!(Dimensionless, 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, From 13f1af747e8f29650bb294a55c3fb44745376997 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 14 Jan 2026 09:34:15 +0000 Subject: [PATCH 16/17] add compare_approx helper and simplify --- src/simulation/investment/appraisal.rs | 49 +++++++++++++++----------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 66222dd04..1f8bb0c44 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -6,7 +6,7 @@ use crate::commodity::Commodity; use crate::finance::{ProfitabilityIndex, lcox, profitability_index}; use crate::model::Model; use crate::time_slice::TimeSliceID; -use crate::units::{Activity, Capacity, Dimensionless, Money, MoneyPerActivity, MoneyPerCapacity}; +use crate::units::{Activity, Capacity, Money, MoneyPerActivity, MoneyPerCapacity}; use anyhow::Result; use costs::annual_fixed_cost; use erased_serde::Serialize as ErasedSerialize; @@ -21,8 +21,32 @@ mod costs; mod optimisation; use coefficients::ObjectiveCoefficients; use float_cmp::approx_eq; +use float_cmp::{ApproxEq, F64Margin}; use optimisation::perform_optimisation; +/// Compares two values with approximate equality checking. +/// +/// Returns `Ordering::Equal` if the values are approximately equal +/// according to the default floating-point margin, otherwise returns +/// their relative ordering based on `a.partial_cmp(&b)`. +/// +/// This is useful when comparing floating-point-based types where exact +/// equality may not be appropriate due to numerical precision limitations. +/// +/// # Panics +/// +/// Panics if `partial_cmp` returns `None` (i.e., if either value is NaN). +fn compare_approx(a: T, b: T) -> Ordering +where + T: Copy + PartialOrd + ApproxEq, +{ + if a.approx_eq(b, F64Margin::default()) { + Ordering::Equal + } else { + a.partial_cmp(&b).unwrap() + } +} + /// The output of investment appraisal required to compare potential investment decisions pub struct AppraisalOutput { /// The asset being appraised @@ -114,12 +138,7 @@ impl ComparableMetric for LCOXMetric { .downcast_ref::() .expect("Cannot compare metrics of different types"); - if approx_eq!(MoneyPerActivity, self.cost, other.cost) { - Ordering::Equal - } else { - // Lower cost is better - self.cost.partial_cmp(&other.cost).unwrap() - } + compare_approx(self.cost, other.cost) } fn as_any(&self) -> &dyn Any { @@ -142,7 +161,7 @@ impl NPVMetric { /// Returns true if this metric represents a zero fixed cost case. fn is_zero_fixed_cost(&self) -> bool { - self.0.annualised_fixed_cost == Money(0.0) + approx_eq!(Money, self.0.annualised_fixed_cost, Money(0.0)) } } @@ -170,23 +189,13 @@ impl ComparableMetric for NPVMetric { (true, true) => { let self_surplus = self.0.total_annualised_surplus; let other_surplus = other.0.total_annualised_surplus; - - if approx_eq!(Money, self_surplus, other_surplus) { - Ordering::Equal - } else { - other_surplus.partial_cmp(&self_surplus).unwrap() - } + compare_approx(other_surplus, self_surplus) } // Both have non-zero fixed cost: compare profitability index (higher is better) (false, false) => { let self_pi = self.0.value(); let other_pi = other.0.value(); - - if approx_eq!(Dimensionless, self_pi, other_pi) { - Ordering::Equal - } else { - other_pi.partial_cmp(&self_pi).unwrap() - } + compare_approx(other_pi, self_pi) } // Zero fixed cost is always better than non-zero fixed cost (true, false) => Ordering::Less, From 4da01556aff28ed9e0a2bd0ec4aa0b60eac160d9 Mon Sep 17 00:00:00 2001 From: Aurash Karimi Date: Wed, 14 Jan 2026 10:30:48 +0000 Subject: [PATCH 17/17] add tests for metric compare methods --- src/simulation/investment/appraisal.rs | 108 +++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 1f8bb0c44..54b8dff18 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -324,3 +324,111 @@ pub fn appraise_investment( }; appraisal_method(model, asset, max_capacity, commodity, coefficients, demand) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::finance::ProfitabilityIndex; + use crate::units::{Money, MoneyPerActivity}; + + #[test] + fn lcox_compare_equal() { + let metric1 = LCOXMetric::new(MoneyPerActivity(10.0)); + let metric2 = LCOXMetric::new(MoneyPerActivity(10.0)); + + assert_eq!(metric1.compare(&metric2), Ordering::Equal); + } + + #[test] + fn lcox_compare_less_is_better() { + let metric1 = LCOXMetric::new(MoneyPerActivity(5.0)); + let metric2 = LCOXMetric::new(MoneyPerActivity(10.0)); + + // metric1 has lower cost, so it's better (Less) + assert_eq!(metric1.compare(&metric2), Ordering::Less); + } + + #[test] + fn lcox_compare_greater_is_worse() { + let metric1 = LCOXMetric::new(MoneyPerActivity(15.0)); + let metric2 = LCOXMetric::new(MoneyPerActivity(10.0)); + + // metric1 has higher cost, so it's worse (Greater) + assert_eq!(metric1.compare(&metric2), Ordering::Greater); + } + + #[test] + fn npv_compare_both_zero_fixed_cost() { + let metric1 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(100.0), + annualised_fixed_cost: Money(0.0), + }); + let metric2 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(50.0), + annualised_fixed_cost: Money(0.0), + }); + + // Compare by surplus: metric1 (100) is better than metric2 (50) + assert_eq!(metric1.compare(&metric2), Ordering::Less); + } + + #[test] + fn npv_compare_both_zero_fixed_cost_equal() { + let metric1 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(100.0), + annualised_fixed_cost: Money(0.0), + }); + let metric2 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(100.0), + annualised_fixed_cost: Money(0.0), + }); + + assert_eq!(metric1.compare(&metric2), Ordering::Equal); + } + + #[test] + fn npv_compare_zero_vs_nonzero_fixed_cost() { + let metric_zero = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(10.0), + annualised_fixed_cost: Money(0.0), + }); + let metric_nonzero = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(1000.0), + annualised_fixed_cost: Money(100.0), + }); + + // Zero fixed cost is always better + assert_eq!(metric_zero.compare(&metric_nonzero), Ordering::Less); + assert_eq!(metric_nonzero.compare(&metric_zero), Ordering::Greater); + } + + #[test] + fn npv_compare_both_nonzero_fixed_cost() { + let metric1 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(200.0), + annualised_fixed_cost: Money(100.0), + }); + let metric2 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(150.0), + annualised_fixed_cost: Money(100.0), + }); + + // Compare by profitability index: 200/100 = 2.0 vs 150/100 = 1.5 + // metric1 is better (higher PI) + assert_eq!(metric1.compare(&metric2), Ordering::Less); + } + + #[test] + fn npv_compare_both_nonzero_fixed_cost_equal() { + let metric1 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(200.0), + annualised_fixed_cost: Money(100.0), + }); + let metric2 = NPVMetric::new(ProfitabilityIndex { + total_annualised_surplus: Money(200.0), + annualised_fixed_cost: Money(100.0), + }); + + assert_eq!(metric1.compare(&metric2), Ordering::Equal); + } +}