diff --git a/src/finance.rs b/src/finance.rs index 495baa54b..bdb296576 100644 --- a/src/finance.rs +++ b/src/finance.rs @@ -74,12 +74,14 @@ pub fn profitability_index( } /// Calculates annual LCOX based on capacity and activity. +/// +/// If the total activity is zero, then it returns `None`, otherwise `Some` LCOX value. pub fn lcox( capacity: Capacity, annual_fixed_cost: MoneyPerCapacity, activity: &IndexMap, activity_costs: &IndexMap, -) -> MoneyPerActivity { +) -> Option { // Calculate the annualised fixed costs let annualised_fixed_cost = annual_fixed_cost * capacity; @@ -92,7 +94,8 @@ pub fn lcox( total_activity_costs += activity_cost * *activity; } - (annualised_fixed_cost + total_activity_costs) / total_activity + (total_activity > Activity(0.0)) + .then(|| (annualised_fixed_cost + total_activity_costs) / total_activity) } #[cfg(test)] @@ -223,20 +226,26 @@ mod tests { 100.0, 50.0, vec![("winter", "day", 10.0), ("summer", "night", 20.0)], vec![("winter", "day", 5.0), ("summer", "night", 3.0)], - 170.33333333333334 // (100*50 + 10*5 + 20*3) / (10+20) = 5110/30 + Some(170.33333333333334) // (100*50 + 10*5 + 20*3) / (10+20) = 5110/30 )] #[case( 50.0, 100.0, vec![("winter", "day", 25.0)], vec![("winter", "day", 0.0)], - 200.0 // (50*100 + 25*0) / 25 = 5000/25 + Some(200.0) // (50*100 + 25*0) / 25 = 5000/25 + )] + #[case( + 50.0, 100.0, + vec![("winter", "day", 0.0)], + vec![("winter", "day", 0.0)], + None // (50*0 + 25*0) / 0 = not feasible )] fn lcox_works( #[case] capacity: f64, #[case] annual_fixed_cost: f64, #[case] activity_data: Vec<(&str, &str, f64)>, #[case] cost_data: Vec<(&str, &str, f64)>, - #[case] expected: f64, + #[case] expected: Option, ) { let activity = activity_data .into_iter() @@ -271,7 +280,7 @@ mod tests { &activity_costs, ); - let expected = MoneyPerActivity(expected); - assert_approx_eq!(MoneyPerActivity, result, expected); + let expected = expected.map(MoneyPerActivity); + assert_approx_eq!(Option, result, expected); } } diff --git a/src/fixture.rs b/src/fixture.rs index a3ef113d5..d959ba997 100644 --- a/src/fixture.rs +++ b/src/fixture.rs @@ -382,20 +382,18 @@ pub fn time_slice_info2() -> TimeSliceInfo { pub fn appraisal_output(asset: Asset, time_slice: TimeSliceID) -> 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: AssetCapacity::Continuous(Capacity(42.0)), - coefficients: ObjectiveCoefficients { + coefficients: Rc::new(ObjectiveCoefficients { capacity_coefficient: MoneyPerCapacity(2.14), activity_coefficients, unmet_demand_coefficient: MoneyPerFlow(10000.0), - }, + }), activity, - demand, unmet_demand, - metric: Box::new(LCOXMetric::new(MoneyPerActivity(4.14))), + metric: Some(Box::new(LCOXMetric::new(MoneyPerActivity(4.14)))), } } diff --git a/src/output.rs b/src/output.rs index edd3df829..5d1f3f84a 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: Option, } /// Represents the appraisal results in a row of the appraisal results CSV file @@ -453,7 +453,7 @@ impl DebugDataWriter { region_id: result.asset.region_id().clone(), capacity: result.capacity.total_capacity(), capacity_coefficient: result.coefficients.capacity_coefficient, - metric: result.metric.value(), + metric: result.metric.as_ref().map(|m| m.value()), }; self.appraisal_results_writer.serialize(row)?; } @@ -467,11 +467,12 @@ impl DebugDataWriter { milestone_year: u32, run_description: &str, appraisal_results: &[AppraisalOutput], + demand: &IndexMap, ) -> Result<()> { for result in appraisal_results { for (time_slice, activity) in &result.activity { let activity_coefficient = result.coefficients.activity_coefficients[time_slice]; - let demand = result.demand[time_slice]; + let demand = demand[time_slice]; let unmet_demand = result.unmet_demand[time_slice]; let row = AppraisalResultsTimeSliceRow { milestone_year, @@ -564,6 +565,7 @@ impl DataWriter { milestone_year: u32, run_description: &str, appraisal_results: &[AppraisalOutput], + demand: &IndexMap, ) -> Result<()> { if let Some(wtr) = &mut self.debug_writer { wtr.write_appraisal_results(milestone_year, run_description, appraisal_results)?; @@ -571,6 +573,7 @@ impl DataWriter { milestone_year, run_description, appraisal_results, + demand, )?; } @@ -986,7 +989,7 @@ mod tests { region_id: asset.region_id().clone(), capacity: Capacity(42.0), capacity_coefficient: MoneyPerCapacity(2.14), - metric: 4.14, + metric: Some(4.14), }; let records: Vec = csv::Reader::from_path(dir.path().join(APPRAISAL_RESULTS_FILE_NAME)) @@ -1006,6 +1009,7 @@ mod tests { let milestone_year = 2020; let run_description = "test_run".to_string(); let dir = tempdir().unwrap(); + let demand = indexmap! {time_slice.clone() => Flow(100.0) }; // Write appraisal time slice results { @@ -1015,6 +1019,7 @@ mod tests { milestone_year, &run_description, &[appraisal_output], + &demand, ) .unwrap(); writer.flush().unwrap(); diff --git a/src/simulation/investment.rs b/src/simulation/investment.rs index 2a2f13157..688bd524d 100644 --- a/src/simulation/investment.rs +++ b/src/simulation/investment.rs @@ -796,6 +796,7 @@ fn select_best_assets( year, &format!("{} {} round {}", &commodity.id, &agent.id, round), &outputs_for_opts, + &demand, )?; sort_appraisal_outputs_by_investment_priority(&mut outputs_for_opts); diff --git a/src/simulation/investment/appraisal.rs b/src/simulation/investment/appraisal.rs index 906bff3cb..7fbf42d68 100644 --- a/src/simulation/investment/appraisal.rs +++ b/src/simulation/investment/appraisal.rs @@ -11,9 +11,11 @@ use anyhow::Result; use costs::annual_fixed_cost; use erased_serde::Serialize as ErasedSerialize; use indexmap::IndexMap; +use optimisation::ResultsMap; use serde::Serialize; use std::any::Any; use std::cmp::Ordering; +use std::rc::Rc; pub mod coefficients; mod constraints; @@ -58,14 +60,28 @@ pub struct AppraisalOutput { /// The hypothetical unmet demand following investment in this asset pub unmet_demand: DemandMap, /// The comparison metric to compare investment decisions - pub metric: Box, + pub metric: Option>, /// Capacity and activity coefficients used in the appraisal - pub coefficients: ObjectiveCoefficients, - /// Demand profile used in the appraisal - pub demand: DemandMap, + pub coefficients: Rc, } impl AppraisalOutput { + /// Create a new `AppraisalOutput` + fn new( + asset: AssetRef, + results: ResultsMap, + metric: Option, + coefficients: Rc, + ) -> Self { + Self { + asset, + capacity: results.capacity, + activity: results.activity, + unmet_demand: results.unmet_demand, + metric: metric.map(|m| Box::new(m) as Box), + coefficients, + } + } /// Compare this appraisal to another on the basis of the comparison metric. /// /// Note that if the metrics are approximately equal (as determined by the [`approx_eq!`] macro) @@ -76,10 +92,23 @@ impl AppraisalOutput { /// 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.is_valid() && other.is_valid(), + "Cannot compare non-valid outputs" ); - self.metric.compare(other.metric.as_ref()) + + // We've already checked the metrics aren't `None` in `is_valid` + self.metric + .as_ref() + .unwrap() + .compare(other.metric.as_ref().unwrap().as_ref()) + } + + /// Whether this [`AppraisalOutput`] is a valid output. + /// + /// Specifically, it checks whether the metric is a valid value (not `None`) and that the + /// calculated capacity is greater than zero. + pub fn is_valid(&self) -> bool { + self.metric.is_some() && self.capacity.total_capacity() > Capacity(0.0) } } @@ -225,7 +254,7 @@ fn calculate_lcox( asset: &AssetRef, max_capacity: Option, commodity: &Commodity, - coefficients: &ObjectiveCoefficients, + coefficients: &Rc, demand: &DemandMap, ) -> Result { let results = perform_optimisation( @@ -245,15 +274,12 @@ fn calculate_lcox( &coefficients.activity_coefficients, ); - Ok(AppraisalOutput { - asset: asset.clone(), - capacity: results.capacity, - activity: results.activity, - unmet_demand: results.unmet_demand, - metric: Box::new(LCOXMetric::new(cost_index)), - coefficients: coefficients.clone(), - demand: demand.clone(), - }) + Ok(AppraisalOutput::new( + asset.clone(), + results, + cost_index.map(LCOXMetric::new), + coefficients.clone(), + )) } /// Calculate NPV for a hypothetical investment in the given asset. @@ -266,7 +292,7 @@ fn calculate_npv( asset: &AssetRef, max_capacity: Option, commodity: &Commodity, - coefficients: &ObjectiveCoefficients, + coefficients: &Rc, demand: &DemandMap, ) -> Result { let results = perform_optimisation( @@ -292,15 +318,12 @@ fn calculate_npv( &coefficients.activity_coefficients, ); - Ok(AppraisalOutput { - asset: asset.clone(), - capacity: results.capacity, - activity: results.activity, - unmet_demand: results.unmet_demand, - metric: Box::new(NPVMetric::new(profitability_index)), - coefficients: coefficients.clone(), - demand: demand.clone(), - }) + Ok(AppraisalOutput::new( + asset.clone(), + results, + Some(NPVMetric::new(profitability_index)), + coefficients.clone(), + )) } /// Appraise the given investment with the specified objective type @@ -315,7 +338,7 @@ pub fn appraise_investment( max_capacity: Option, commodity: &Commodity, objective_type: &ObjectiveType, - coefficients: &ObjectiveCoefficients, + coefficients: &Rc, demand: &DemandMap, ) -> Result { let appraisal_method = match objective_type { @@ -342,12 +365,12 @@ fn compare_asset_fallback(asset1: &Asset, asset2: &Asset) -> Ordering { /// are preferred over uncommissioned assets, and newer assets are preferred over older /// ones. The function does not guarantee that all ties will be resolved. /// -/// Assets with zero capacity are filtered out before sorting, -/// as their metric would be `NaN` and could cause the program to panic. So the length -/// of the returned vector may be less than the input. -/// +/// Before sorting, outputs are filtered using [`AppraisalOutput::is_valid`], which +/// excludes entries with invalid metrics (e.g. `None`) as well as zero capacity. This +/// avoids meaningless or `NaN` appraisal metrics that could cause the program to panic, +/// so the length of the returned vector may be less than the input. pub fn sort_appraisal_outputs_by_investment_priority(outputs_for_opts: &mut Vec) { - outputs_for_opts.retain(|output| output.capacity.total_capacity() > Capacity(0.0)); + outputs_for_opts.retain(AppraisalOutput::is_valid); outputs_for_opts.sort_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), @@ -530,6 +553,14 @@ mod tests { assert!(compare_asset_fallback(&asset2, &asset3).is_gt()); } + fn objective_coeffs() -> Rc { + Rc::new(ObjectiveCoefficients { + capacity_coefficient: MoneyPerCapacity(0.0), + activity_coefficients: IndexMap::new(), + unmet_demand_coefficient: MoneyPerFlow(0.0), + }) + } + /// Creates appraisal from corresponding assets and metrics /// /// # Panics @@ -551,15 +582,10 @@ mod tests { .map(|(asset, metric)| AppraisalOutput { asset: AssetRef::from(asset), capacity: AssetCapacity::Continuous(Capacity(10.0)), - coefficients: ObjectiveCoefficients { - capacity_coefficient: MoneyPerCapacity(0.0), - activity_coefficients: IndexMap::new(), - unmet_demand_coefficient: MoneyPerFlow(0.0), - }, + coefficients: objective_coeffs(), activity: IndexMap::new(), - demand: IndexMap::new(), unmet_demand: IndexMap::new(), - metric, + metric: Some(metric), }) .collect() } @@ -587,9 +613,9 @@ mod tests { appraisal_outputs_with_investment_priority_invariant_to_assets(metrics, &asset); sort_appraisal_outputs_by_investment_priority(&mut outputs); - assert_approx_eq!(f64, outputs[0].metric.value(), 3.0); // Best (lowest) - assert_approx_eq!(f64, outputs[1].metric.value(), 5.0); - assert_approx_eq!(f64, outputs[2].metric.value(), 7.0); // Worst (highest) + assert_approx_eq!(f64, outputs[0].metric.as_ref().unwrap().value(), 3.0); // Best (lowest) + assert_approx_eq!(f64, outputs[1].metric.as_ref().unwrap().value(), 5.0); + assert_approx_eq!(f64, outputs[2].metric.as_ref().unwrap().value(), 7.0); // Worst (highest) } /// Test sorting by NPV profitability index when invariant to asset properties @@ -615,9 +641,9 @@ mod tests { sort_appraisal_outputs_by_investment_priority(&mut outputs); // Higher profitability index is better, so should be sorted: 3.0, 2.0, 1.5 - assert_approx_eq!(f64, outputs[0].metric.value(), 3.0); // Best (highest PI) - assert_approx_eq!(f64, outputs[1].metric.value(), 2.0); - assert_approx_eq!(f64, outputs[2].metric.value(), 1.5); // Worst (lowest PI) + assert_approx_eq!(f64, outputs[0].metric.as_ref().unwrap().value(), 3.0); // Best (highest PI) + assert_approx_eq!(f64, outputs[1].metric.as_ref().unwrap().value(), 2.0); + assert_approx_eq!(f64, outputs[2].metric.as_ref().unwrap().value(), 1.5); // Worst (lowest PI) } /// Test that NPV metrics with zero annual fixed cost are prioritised above all others @@ -647,9 +673,9 @@ mod tests { sort_appraisal_outputs_by_investment_priority(&mut outputs); // Zero AFC should be first despite lower absolute surplus value - assert_approx_eq!(f64, outputs[0].metric.value(), 50.0); // Zero AFC (uses surplus) - assert_approx_eq!(f64, outputs[1].metric.value(), 10.0); // PI = 1000/100 - assert_approx_eq!(f64, outputs[2].metric.value(), 10.0); // PI = 500/50 + assert_approx_eq!(f64, outputs[0].metric.as_ref().unwrap().value(), 50.0); // Zero AFC (uses surplus) + assert_approx_eq!(f64, outputs[1].metric.as_ref().unwrap().value(), 10.0); // PI = 1000/100 + assert_approx_eq!(f64, outputs[2].metric.as_ref().unwrap().value(), 10.0); // PI = 500/50 } /// Test that mixing LCOX and NPV metrics causes a runtime panic during comparison @@ -863,16 +889,21 @@ mod tests { sort_appraisal_outputs_by_investment_priority(&mut outputs); // non-commissioned asset prioritised because it has a slightly better metric - assert_approx_eq!(f64, outputs[0].metric.value(), best_metric_value); + assert_approx_eq!( + f64, + outputs[0].metric.as_ref().unwrap().value(), + best_metric_value + ); } /// Test that appraisal outputs with zero capacity are filtered out during sorting. #[rstest] fn appraisal_sort_filters_zero_capacity_outputs(asset: Asset) { - let metrics: Vec> = vec![ - Box::new(LCOXMetric::new(MoneyPerActivity(f64::NAN))), - Box::new(LCOXMetric::new(MoneyPerActivity(f64::NAN))), - Box::new(LCOXMetric::new(MoneyPerActivity(f64::NAN))), + let metric = LCOXMetric::new(MoneyPerActivity(1.0)); + let metrics = [ + Box::new(metric.clone()), + Box::new(metric.clone()), + Box::new(metric), ]; // Create outputs with zero capacity @@ -881,15 +912,10 @@ mod tests { .map(|metric| AppraisalOutput { asset: AssetRef::from(asset.clone()), capacity: AssetCapacity::Continuous(Capacity(0.0)), - coefficients: ObjectiveCoefficients { - capacity_coefficient: MoneyPerCapacity(0.0), - activity_coefficients: IndexMap::new(), - unmet_demand_coefficient: MoneyPerFlow(0.0), - }, + coefficients: objective_coeffs(), activity: IndexMap::new(), - demand: IndexMap::new(), unmet_demand: IndexMap::new(), - metric, + metric: Some(metric), }) .collect(); @@ -898,4 +924,23 @@ mod tests { // All zero capacity outputs should be filtered out assert_eq!(outputs.len(), 0); } + + /// Test that appraisal outputs with an invalid metric are filtered out + #[rstest] + fn appraisal_sort_filters_invalid_metric(asset: Asset) { + let output = AppraisalOutput { + asset: AssetRef::from(asset), + capacity: AssetCapacity::Continuous(Capacity(1.0)), // non-zero capacity + coefficients: objective_coeffs(), + activity: IndexMap::new(), + unmet_demand: IndexMap::new(), + metric: None, + }; + let mut outputs = vec![output]; + + sort_appraisal_outputs_by_investment_priority(&mut outputs); + + // The invalid output should have been filtered out + assert_eq!(outputs.len(), 0); + } } diff --git a/src/simulation/investment/appraisal/coefficients.rs b/src/simulation/investment/appraisal/coefficients.rs index 0c17f2b74..286b93535 100644 --- a/src/simulation/investment/appraisal/coefficients.rs +++ b/src/simulation/investment/appraisal/coefficients.rs @@ -8,6 +8,7 @@ use crate::time_slice::{TimeSliceID, TimeSliceInfo}; use crate::units::{MoneyPerActivity, MoneyPerCapacity, MoneyPerFlow}; use indexmap::IndexMap; use std::collections::HashMap; +use std::rc::Rc; /// Map storing cost coefficients for an asset. /// @@ -31,7 +32,7 @@ pub fn calculate_coefficients_for_assets( assets: &[AssetRef], prices: &CommodityPrices, year: u32, -) -> HashMap { +) -> HashMap> { assets .iter() .map(|asset| { @@ -47,7 +48,7 @@ pub fn calculate_coefficients_for_assets( calculate_coefficients_for_npv(asset, &model.time_slice_info, prices, year) } }; - (asset.clone(), coefficient) + (asset.clone(), Rc::new(coefficient)) }) .collect() }