From 3dcef78badcb7e27f85d4d873eb1610bd7cf70e7 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Thu, 12 Mar 2026 10:22:53 +0000
Subject: [PATCH 01/23] Basic implementation for averaging marginal costs

---
 src/simulation/prices.rs | 407 +++++++++++++++++++++++++++------------
 src/time_slice.rs        |  11 ++
 2 files changed, 293 insertions(+), 125 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 81c2b4f84..1e8c7abfe 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -1,6 +1,6 @@
 //! Code for calculating commodity prices used by the simulation.
 use crate::asset::AssetRef;
-use crate::commodity::{CommodityID, PricingStrategy};
+use crate::commodity::{CommodityID, CommodityMap, PricingStrategy};
 use crate::model::Model;
 use crate::region::RegionID;
 use crate::simulation::optimisation::Solution;
@@ -78,11 +78,13 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
         // Add prices for marginal cost commodities
         if let Some(marginal_set) = pricing_sets.get(&PricingStrategy::MarginalCost) {
             let marginal_cost_prices = calculate_marginal_cost_prices(
-                solution.iter_activity_keys_for_existing(),
+                solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
                 &result,
                 year,
                 marginal_set,
+                &model.commodities,
+                &model.time_slice_info,
             );
             result.extend(marginal_cost_prices);
         }
@@ -92,12 +94,14 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
             let annual_activities =
                 calculate_annual_activities(solution.iter_activity_for_existing());
             let full_cost_prices = calculate_full_cost_prices(
-                solution.iter_activity_keys_for_existing(),
+                solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
                 &annual_activities,
                 &result,
                 year,
                 fullcost_set,
+                &model.commodities,
+                &model.time_slice_info,
             );
             result.extend(full_cost_prices);
         }
@@ -322,6 +326,45 @@ where
     scarcity_prices
 }
 
+/// Helper struct for accumulating weighted marginal costs for a group of (asset, commodity, region,
+/// time_slice_selection) tuples when calculating marginal cost or full cost prices.
+struct GroupAccum {
+    weighted_cost_numerator: MoneyPerFlow,
+    weighted_cost_denominator: Dimensionless,
+    backup_numerator: MoneyPerFlow,
+    backup_denominator: Dimensionless,
+}
+
+impl Default for GroupAccum {
+    fn default() -> Self {
+        Self {
+            weighted_cost_numerator: MoneyPerFlow(0.0),
+            weighted_cost_denominator: Dimensionless(0.0),
+            backup_numerator: MoneyPerFlow(0.0),
+            backup_denominator: Dimensionless(0.0),
+        }
+    }
+}
+
+impl GroupAccum {
+    fn add(&mut self, marginal_cost: MoneyPerFlow, activity: Activity, activity_limit: Activity) {
+        self.weighted_cost_numerator += marginal_cost * Dimensionless(activity.value());
+        self.weighted_cost_denominator += Dimensionless(activity.value());
+        self.backup_numerator += marginal_cost * Dimensionless(activity_limit.value());
+        self.backup_denominator += Dimensionless(activity_limit.value());
+    }
+
+    fn solve(&self) -> Option<MoneyPerFlow> {
+        if self.weighted_cost_denominator > Dimensionless::EPSILON {
+            Some(self.weighted_cost_numerator / self.weighted_cost_denominator)
+        } else if self.backup_denominator > Dimensionless::EPSILON {
+            Some(self.backup_numerator / self.backup_denominator)
+        } else {
+            None
+        }
+    }
+}
+
 /// Calculate marginal cost prices for a set of commodities.
 ///
 /// This pricing strategy aims to incorporate the marginal cost of commodity production into the price.
@@ -356,94 +399,152 @@ where
 /// utilisation (i.e. the single candidate asset that would be most competitive if a small amount of
 /// demand was added).
 ///
-/// Note: this should be similar to the "shadow price" strategy, which is also based on marginal
-/// costs of the most expensive producer, but may be more successful in cases where there are
-/// multiple SED/SVD outputs per asset.
-///
 /// # Arguments
 ///
-/// * `activity_keys_for_existing` - Iterator over activity keys from optimisation solution for
-///   existing assets
-/// * `activity_keys_for_candidates` - Iterator over activity keys from optimisation solution for
-///   candidate assets
+/// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
+///   solution for existing assets
+/// * `activity_keys_for_candidates` - Iterator over `(asset, time_slice)` for candidate assets
 /// * `upstream_prices` - Prices for commodities upstream of the ones we are calculating prices for
 /// * `year` - The year for which prices are being calculated
 /// * `markets_to_price` - Set of markets to calculate marginal prices for
+/// * `commodities` - Map of all commodities (used to look up each commodity's `time_slice_level`)
+/// * `time_slice_info` - Time slice information (used to expand groups to individual time slices)
 ///
 /// # Returns
 ///
 /// A map of marginal cost prices for the specified markets in all time slices
 fn calculate_marginal_cost_prices<'a, I, J>(
-    activity_keys_for_existing: I,
+    activity_for_existing: I,
     activity_keys_for_candidates: J,
     upstream_prices: &CommodityPrices,
     year: u32,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
+    commodities: &CommodityMap,
+    time_slice_info: &TimeSliceInfo,
 ) -> HashMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
 where
-    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
+    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    let mut prices: HashMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow> = HashMap::new();
-
-    // Start by looking at existing assets
-    // Calculate highest marginal cost for each commodity/region/time slice
-    // Keep track of keys with prices - missing keys will be handled by candidates later
-    let mut priced_by_existing = HashSet::new();
-    for (asset, time_slice) in activity_keys_for_existing {
+    // For each (asset, commodity, region, group), accumulate marginal costs weighted by
+    let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
+    for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
+        let activity_limit = *asset
+            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
+            .end();
 
         // Iterate over all the SED/SVD marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
             upstream_prices,
             year,
             time_slice,
-            |commodity_id: &CommodityID| {
-                markets_to_price.contains(&(commodity_id.clone(), region_id.clone()))
-            },
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Update the highest cost for this commodity/region/time slice
-            let key = (commodity_id.clone(), region_id.clone(), time_slice.clone());
-            prices
-                .entry(key.clone())
-                .and_modify(|c| *c = c.max(marginal_cost))
-                .or_insert(marginal_cost);
-            priced_by_existing.insert(key);
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+            let key = (
+                asset.clone(),
+                commodity_id.clone(),
+                region_id.clone(),
+                group,
+            );
+            let accum = existing_accum.entry(key).or_default();
+            accum.add(marginal_cost, activity, activity_limit);
         }
     }
 
-    // Next, look at candidate assets for any markets not covered by existing assets
-    // For these, we take the _lowest_ marginal cost
-    for (asset, time_slice) in activity_keys_for_candidates {
-        let region_id = asset.region_id();
+    // Compute per-group weighted-average marginal cost per asset, then take the max across assets
+    let mut group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
+    let mut priced_groups: HashSet<_> = HashSet::new();
 
-        // Only consider markets not already priced by existing assets
-        let should_process = |cid: &CommodityID| {
-            markets_to_price.contains(&(cid.clone(), region_id.clone()))
-                && !priced_by_existing.contains(&(
-                    cid.clone(),
-                    region_id.clone(),
-                    time_slice.clone(),
-                ))
+    for ((_, commodity_id, region_id, group), accum) in &existing_accum {
+        // Use actual activity weights if any activity occurred in this group; otherwise fall back
+        // to max potential activity weights
+        let Some(avg_cost) = accum.solve() else {
+            continue;
         };
 
-        // Iterate over all the SED/SVD marginal costs for markets we need prices for
+        group_prices
+            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
+            .and_modify(|c| *c = c.max(avg_cost))
+            .or_insert(avg_cost);
+        priced_groups.insert((commodity_id.clone(), region_id.clone(), group.clone()));
+    }
+
+    // Expand each group to individual time slices
+    let mut prices: HashMap<_, MoneyPerFlow> = HashMap::new();
+    for ((commodity_id, region_id, group), group_price) in &group_prices {
+        for (ts, _) in group.iter(time_slice_info) {
+            prices.insert(
+                (commodity_id.clone(), region_id.clone(), ts.clone()),
+                *group_price,
+            );
+        }
+    }
+
+    // Candidate assets: use max potential activity weights, take the min across assets
+    // (price from the most cost-effective candidate)
+    let mut cand_accum: HashMap<_, (MoneyPerFlow, Dimensionless)> = HashMap::new();
+    for (asset, time_slice) in activity_keys_for_candidates {
+        let region_id = asset.region_id();
+        let activity_limit = *asset
+            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
+            .end();
+
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
             upstream_prices,
             year,
             time_slice,
-            |cid: &CommodityID| should_process(cid),
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Update the _lowest_ cost for this commodity/region/time slice
-            let key = (commodity_id.clone(), region_id.clone(), time_slice.clone());
-            prices
-                .entry(key.clone())
-                .and_modify(|c| *c = c.min(marginal_cost))
-                .or_insert(marginal_cost);
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+
+            // Skip groups already covered by existing assets
+            if priced_groups.contains(&(commodity_id.clone(), region_id.clone(), group.clone())) {
+                continue;
+            }
+
+            let key = (
+                asset.clone(),
+                commodity_id.clone(),
+                region_id.clone(),
+                group,
+            );
+            let (cost_sum, weight_sum) = cand_accum
+                .entry(key)
+                .or_insert((MoneyPerFlow(0.0), Dimensionless(0.0)));
+            *cost_sum += marginal_cost * Dimensionless(activity_limit.value());
+            *weight_sum += Dimensionless(activity_limit.value());
+        }
+    }
+
+    // Compute per-group weighted average per candidate, then take the min across candidates
+    let mut cand_group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
+    for ((_, commodity_id, region_id, group), (cost_sum, weight_sum)) in &cand_accum {
+        if *weight_sum < Dimensionless::EPSILON {
+            continue;
+        }
+        let avg_cost = *cost_sum / *weight_sum;
+        cand_group_prices
+            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
+            .and_modify(|c| *c = c.min(avg_cost))
+            .or_insert(avg_cost);
+    }
+
+    // Expand candidate groups to individual time slices
+    for ((commodity_id, region_id, group), group_price) in &cand_group_prices {
+        for (ts, _) in group.iter(time_slice_info) {
+            prices.insert(
+                (commodity_id.clone(), region_id.clone(), ts.clone()),
+                *group_price,
+            );
         }
     }
 
-    // Return the calculated marginal prices
     prices
 }
 
@@ -506,39 +607,37 @@ where
 ///
 /// # Arguments
 ///
-/// * `activity_keys_for_existing` - Iterator over activity keys from optimisation solution for
-///   existing assets
-/// * `activity_keys_for_candidates` - Iterator over activity keys from optimisation solution for
-///   candidate assets
+/// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
+///   solution for existing assets
+/// * `activity_keys_for_candidates` - Iterator over `(asset, time_slice)` for candidate assets
 /// * `annual_activities` - Map of annual activities for each asset computed by
 ///   `calculate_annual_activities`. This only needs to include existing assets.
 /// * `upstream_prices` - Prices for commodities upstream of the ones we are calculating prices for
 /// * `year` - The year for which prices are being calculated
 /// * `markets_to_price` - Set of markets to calculate full cost prices for
+/// * `commodities` - Map of all commodities (used to look up each commodity's `time_slice_level`)
+/// * `time_slice_info` - Time slice information (used to expand groups to individual time slices)
 ///
 /// # Returns
 ///
 /// A map of full cost prices for the specified markets in all time slices
 fn calculate_full_cost_prices<'a, I, J>(
-    activity_keys_for_existing: I,
+    activity_for_existing: I,
     activity_keys_for_candidates: J,
     annual_activities: &HashMap<AssetRef, Activity>,
     upstream_prices: &CommodityPrices,
     year: u32,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
+    commodities: &CommodityMap,
+    time_slice_info: &TimeSliceInfo,
 ) -> HashMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
 where
-    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
+    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    let mut prices: HashMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow> = HashMap::new();
-
-    // Start by looking at existing assets
-    // Calculate highest full cost for each commodity/region/time slice
-    // Keep track of keys with prices - missing keys will be handled by candidates later
-    let mut annual_fixed_costs_cache = HashMap::new();
-    let mut priced_by_existing = HashSet::new();
-    for (asset, time_slice) in activity_keys_for_existing {
+    // For each (asset, commodity, region, group), accumulate marginal costs weighted by
+    let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
+    for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
 
@@ -548,18 +647,9 @@ where
             continue;
         }
 
-        // Only proceed if the asset produces at least one commodity we need prices for
-        if !asset
-            .iter_output_flows()
-            .any(|flow| markets_to_price.contains(&(flow.commodity.id.clone(), region_id.clone())))
-        {
-            continue;
-        }
-
-        // Calculate/cache annual fixed costs for this asset
-        let annual_fixed_costs_per_flow = *annual_fixed_costs_cache
-            .entry(asset.clone())
-            .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activity));
+        let activity_limit = *asset
+            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
+            .end();
 
         // Iterate over all the SED/SVD marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
@@ -568,45 +658,103 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Add annual fixed costs per flow to marginal cost to get full cost
-            let marginal_cost = marginal_cost + annual_fixed_costs_per_flow;
-
-            // Update the highest cost for this commodity/region/time slice
-            let key = (commodity_id.clone(), region_id.clone(), time_slice.clone());
-            prices
-                .entry(key.clone())
-                .and_modify(|c| *c = c.max(marginal_cost))
-                .or_insert(marginal_cost);
-            priced_by_existing.insert(key);
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+            let key = (
+                asset.clone(),
+                commodity_id.clone(),
+                region_id.clone(),
+                group,
+            );
+            let accum = existing_accum.entry(key).or_default();
+            accum.add(marginal_cost, activity, activity_limit);
         }
     }
 
-    // Next, look at candidate assets for any markets not covered by existing assets
-    // For these we assume full utilisation, and take the _lowest_ full cost
+    // Compute per-group weighted-average marginal cost per asset, add fixed costs, then take max
+    let mut group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
+    let mut priced_groups: HashSet<_> = HashSet::new();
+    let mut existing_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
+
+    for ((asset, commodity_id, region_id, group), accum) in &existing_accum {
+        // Use actual activity weights if any activity occurred in this group; otherwise fall back
+        // to max potential activity weights
+        let Some(avg_mc) = accum.solve() else {
+            continue;
+        };
+
+        let annual_fixed_costs_per_flow = *existing_fixed_costs_cache
+            .entry(asset.clone())
+            .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activities[asset]));
+
+        let full_cost = avg_mc + annual_fixed_costs_per_flow;
+        group_prices
+            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
+            .and_modify(|c| *c = c.max(full_cost))
+            .or_insert(full_cost);
+        priced_groups.insert((commodity_id.clone(), region_id.clone(), group.clone()));
+    }
+
+    // Expand each group to individual time slices
+    let mut prices: HashMap<_, MoneyPerFlow> = HashMap::new();
+    for ((commodity_id, region_id, group), group_price) in &group_prices {
+        for (ts, _) in group.iter(time_slice_info) {
+            prices.insert(
+                (commodity_id.clone(), region_id.clone(), ts.clone()),
+                *group_price,
+            );
+        }
+    }
+
+    // Candidate assets: assume full dispatch, use limit weights, take the min across candidates
+    let mut cand_accum: HashMap<_, (MoneyPerFlow, Dimensionless)> = HashMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
+        let activity_limit = *asset
+            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
+            .end();
 
-        // Only consider markets not already priced by existing assets
-        let should_process = |cid: &CommodityID| {
-            markets_to_price.contains(&(cid.clone(), region_id.clone()))
-                && !priced_by_existing.contains(&(
-                    cid.clone(),
-                    region_id.clone(),
-                    time_slice.clone(),
-                ))
-        };
+        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
+            upstream_prices,
+            year,
+            time_slice,
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
+        ) {
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+
+            // Skip groups already covered by existing assets
+            if priced_groups.contains(&(commodity_id.clone(), region_id.clone(), group.clone())) {
+                continue;
+            }
 
-        // Only proceed if the asset produces at least one commodity we need prices for
-        if !asset
-            .iter_output_flows()
-            .any(|flow| should_process(&flow.commodity.id))
-        {
+            let key = (
+                asset.clone(),
+                commodity_id.clone(),
+                region_id.clone(),
+                group,
+            );
+            let (cost_sum, weight_sum) = cand_accum
+                .entry(key)
+                .or_insert((MoneyPerFlow(0.0), Dimensionless(0.0)));
+            *cost_sum += marginal_cost * Dimensionless(activity_limit.value());
+            *weight_sum += Dimensionless(activity_limit.value());
+        }
+    }
+
+    // Compute per-group weighted average, add fixed costs, take the min across candidates
+    let mut cand_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
+    let mut cand_group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
+    for ((asset, commodity_id, region_id, group), (cost_sum, weight_sum)) in &cand_accum {
+        if *weight_sum < Dimensionless::EPSILON {
             continue;
         }
+        let avg_mc = *cost_sum / *weight_sum;
 
-        // Calculate/cache annual fixed cost per flow for this asset assuming full dispatch
-        // (bound by the activity limits of the asset)
-        let annual_fixed_costs_per_flow = *annual_fixed_costs_cache
+        // For candidates, assume full annual dispatch
+        let annual_fixed_costs_per_flow = *cand_fixed_costs_cache
             .entry(asset.clone())
             .or_insert_with(|| {
                 asset.get_annual_fixed_costs_per_flow(
@@ -616,26 +764,23 @@ where
                 )
             });
 
-        // Iterate over all the SED/SVD marginal costs for markets we need prices for
-        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
-            year,
-            time_slice,
-            |cid: &CommodityID| should_process(cid),
-        ) {
-            // Add annual fixed costs per flow to marginal cost to get full cost
-            let full_cost = marginal_cost + annual_fixed_costs_per_flow;
+        let full_cost = avg_mc + annual_fixed_costs_per_flow;
+        cand_group_prices
+            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
+            .and_modify(|c| *c = c.min(full_cost))
+            .or_insert(full_cost);
+    }
 
-            // Update the _lowest_ cost for this commodity/region/time slice
-            let key = (commodity_id.clone(), region_id.clone(), time_slice.clone());
-            prices
-                .entry(key)
-                .and_modify(|c| *c = c.min(full_cost))
-                .or_insert(full_cost);
+    // Expand candidate groups to individual time slices
+    for ((commodity_id, region_id, group), group_price) in &cand_group_prices {
+        for (ts, _) in group.iter(time_slice_info) {
+            prices.insert(
+                (commodity_id.clone(), region_id.clone(), ts.clone()),
+                *group_price,
+            );
         }
     }
 
-    // Return the calculated full cost prices
     prices
 }
 
@@ -644,7 +789,7 @@ mod tests {
     use super::*;
     use crate::asset::Asset;
     use crate::asset::AssetRef;
-    use crate::commodity::{Commodity, CommodityID};
+    use crate::commodity::{Commodity, CommodityID, CommodityMap};
     use crate::fixture::{
         commodity_id, other_commodity, region_id, sed_commodity, time_slice, time_slice_info,
     };
@@ -830,7 +975,11 @@ mod tests {
         markets.insert((b.id.clone(), region_id.clone()));
         markets.insert((c.id.clone(), region_id.clone()));
 
-        let existing = vec![(&asset_ref, &time_slice)];
+        let mut commodities = CommodityMap::new();
+        commodities.insert(b.id.clone(), Rc::new(b.clone()));
+        commodities.insert(c.id.clone(), Rc::new(c.clone()));
+
+        let existing = vec![(&asset_ref, &time_slice, Activity(1.0))];
         let candidates = Vec::new();
 
         let prices = calculate_marginal_cost_prices(
@@ -839,6 +988,8 @@ mod tests {
             &shadow_prices,
             2015u32,
             &markets,
+            &commodities,
+            &time_slice_info,
         );
 
         assert_price_approx(
@@ -906,7 +1057,11 @@ mod tests {
         markets.insert((b.id.clone(), region_id.clone()));
         markets.insert((c.id.clone(), region_id.clone()));
 
-        let existing = vec![(&asset_ref, &time_slice)];
+        let mut commodities = CommodityMap::new();
+        commodities.insert(b.id.clone(), Rc::new(b.clone()));
+        commodities.insert(c.id.clone(), Rc::new(c.clone()));
+
+        let existing = vec![(&asset_ref, &time_slice, Activity(2.0))];
         let candidates = Vec::new();
 
         let mut annual_activities = HashMap::new();
@@ -919,6 +1074,8 @@ mod tests {
             &shadow_prices,
             2015u32,
             &markets,
+            &commodities,
+            &time_slice_info,
         );
 
         assert_price_approx(&prices, &b.id, &region_id, &time_slice, MoneyPerFlow(5.0));
diff --git a/src/time_slice.rs b/src/time_slice.rs
index 6783b9aeb..3d72d80ff 100644
--- a/src/time_slice.rs
+++ b/src/time_slice.rs
@@ -206,6 +206,17 @@ pub enum TimeSliceLevel {
     Annual,
 }
 
+impl TimeSliceLevel {
+    /// Get the [`TimeSliceSelection`] containing the given [`TimeSliceID`] at this level.
+    pub fn containing_selection(&self, ts: &TimeSliceID) -> TimeSliceSelection {
+        match self {
+            Self::Annual => TimeSliceSelection::Annual,
+            Self::Season => TimeSliceSelection::Season(ts.season.clone()),
+            Self::DayNight => TimeSliceSelection::Single(ts.clone()),
+        }
+    }
+}
+
 /// Information about the time slices in the simulation, including names and durations
 #[derive(PartialEq, Debug)]
 pub struct TimeSliceInfo {

From 6e3559dbebf394a25169ff431d50ba8a6c0ff0e1 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Thu, 12 Mar 2026 14:51:47 +0000
Subject: [PATCH 02/23] Better use of GroupAccum and more comments

---
 src/simulation/prices.rs | 100 ++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 37 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 1e8c7abfe..99fad537c 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -327,10 +327,16 @@ where
 }
 
 /// Helper struct for accumulating weighted marginal costs for a group of (asset, commodity, region,
-/// time_slice_selection) tuples when calculating marginal cost or full cost prices.
+/// time slice selection) tuples when calculating marginal cost or full cost prices.
+///
+/// For seasonal/annual commodities, marginal costs are weighted by activity (or the activity limit
+/// for assets with no activity) to get a time slice-weighted average marginal cost for the group.
 struct GroupAccum {
+    // Sum of (marginal cost * activity) across all tuples in the group
     weighted_cost_numerator: MoneyPerFlow,
+    // Sum of activity across all tuples in the group
     weighted_cost_denominator: Dimensionless,
+    // Backup numerator and denominator for the case where there's no activity across the group
     backup_numerator: MoneyPerFlow,
     backup_denominator: Dimensionless,
 }
@@ -347,6 +353,7 @@ impl Default for GroupAccum {
 }
 
 impl GroupAccum {
+    /// Add a marginal cost to the accumulator
     fn add(&mut self, marginal_cost: MoneyPerFlow, activity: Activity, activity_limit: Activity) {
         self.weighted_cost_numerator += marginal_cost * Dimensionless(activity.value());
         self.weighted_cost_denominator += Dimensionless(activity.value());
@@ -354,6 +361,8 @@ impl GroupAccum {
         self.backup_denominator += Dimensionless(activity_limit.value());
     }
 
+    /// Solve the weighted average marginal cost for the group, using the backup weights (activity
+    /// limits) if there's no activity across the group. If both denominators are zero, return None.
     fn solve(&self) -> Option<MoneyPerFlow> {
         if self.weighted_cost_denominator > Dimensionless::EPSILON {
             Some(self.weighted_cost_numerator / self.weighted_cost_denominator)
@@ -399,6 +408,11 @@ impl GroupAccum {
 /// utilisation (i.e. the single candidate asset that would be most competitive if a small amount of
 /// demand was added).
 ///
+/// For commodities with seasonal/annual time slice levels, marginal costs are weighted by
+/// activity (or the maximum potential activity for candidates) to get a time slice-weighted average
+/// marginal cost for each asset, before taking the max across assets. Consequentially, the price of
+/// these commodities is flat within each season/year.
+///
 /// # Arguments
 ///
 /// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
@@ -426,15 +440,19 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // For each (asset, commodity, region, group), accumulate marginal costs weighted by
+    // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
+    // commodities, marginal costs are weighted by activity (or the activity limit for assets with
+    // no activity)
     let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
+
+        // Get activity limits: used as a backup weight if no activity across the group
         let activity_limit = *asset
             .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
             .end();
 
-        // Iterate over all the SED/SVD marginal costs for commodities we need prices for
+        // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
             upstream_prices,
             year,
@@ -458,14 +476,13 @@ where
     // Compute per-group weighted-average marginal cost per asset, then take the max across assets
     let mut group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
     let mut priced_groups: HashSet<_> = HashSet::new();
-
     for ((_, commodity_id, region_id, group), accum) in &existing_accum {
-        // Use actual activity weights if any activity occurred in this group; otherwise fall back
-        // to max potential activity weights
+        // Solve the weighted average marginal cost for this group
         let Some(avg_cost) = accum.solve() else {
             continue;
         };
 
+        // Take the max across assets for each group
         group_prices
             .entry((commodity_id.clone(), region_id.clone(), group.clone()))
             .and_modify(|c| *c = c.max(avg_cost))
@@ -484,15 +501,17 @@ where
         }
     }
 
-    // Candidate assets: use max potential activity weights, take the min across assets
-    // (price from the most cost-effective candidate)
-    let mut cand_accum: HashMap<_, (MoneyPerFlow, Dimensionless)> = HashMap::new();
+    // Candidate assets: weight marginal costs by activity limits (i.e. assume full utilisation)
+    let mut cand_accum: HashMap<_, GroupAccum> = HashMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
+
+        // Get activity limits: used as a to weight marginal costs for seasonal/annual commodities
         let activity_limit = *asset
             .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
             .end();
 
+        // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
             upstream_prices,
             year,
@@ -514,21 +533,18 @@ where
                 region_id.clone(),
                 group,
             );
-            let (cost_sum, weight_sum) = cand_accum
-                .entry(key)
-                .or_insert((MoneyPerFlow(0.0), Dimensionless(0.0)));
-            *cost_sum += marginal_cost * Dimensionless(activity_limit.value());
-            *weight_sum += Dimensionless(activity_limit.value());
+            let accum = cand_accum.entry(key).or_default();
+            accum.add(marginal_cost, Activity(0.0), activity_limit);
         }
     }
 
     // Compute per-group weighted average per candidate, then take the min across candidates
+    // (i.e. the single most competitive candidate if a small amount of demand was added)
     let mut cand_group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
-    for ((_, commodity_id, region_id, group), (cost_sum, weight_sum)) in &cand_accum {
-        if *weight_sum < Dimensionless::EPSILON {
+    for ((_, commodity_id, region_id, group), accum) in &cand_accum {
+        let Some(avg_cost) = accum.solve() else {
             continue;
-        }
-        let avg_cost = *cost_sum / *weight_sum;
+        };
         cand_group_prices
             .entry((commodity_id.clone(), region_id.clone(), group.clone()))
             .and_modify(|c| *c = c.min(avg_cost))
@@ -605,6 +621,11 @@ where
 /// maximum possible dispatch (i.e. the single candidate asset that would be most competitive if a
 /// small amount of demand was added).
 ///
+/// For commodities with seasonal/annual time slice levels, costs are weighted by activity (or the
+/// maximum potential activity for candidates) to get a time slice-weighted average cost for each
+/// asset, before taking the max across assets. Consequentially, the price of these commodities is
+/// flat within each season/year.
+///
 /// # Arguments
 ///
 /// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
@@ -635,7 +656,9 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // For each (asset, commodity, region, group), accumulate marginal costs weighted by
+    // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
+    // commodities, marginal costs are weighted by activity (or the activity limit for assets with
+    // no activity)
     let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
@@ -647,11 +670,12 @@ where
             continue;
         }
 
+        // Get activity limits: used as a backup weight if no activity across the group
         let activity_limit = *asset
             .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
             .end();
 
-        // Iterate over all the SED/SVD marginal costs for commodities we need prices for
+        // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
             upstream_prices,
             year,
@@ -676,19 +700,19 @@ where
     let mut group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
     let mut priced_groups: HashSet<_> = HashSet::new();
     let mut existing_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
-
     for ((asset, commodity_id, region_id, group), accum) in &existing_accum {
-        // Use actual activity weights if any activity occurred in this group; otherwise fall back
-        // to max potential activity weights
+        // Solve the weighted average marginal cost for this group
         let Some(avg_mc) = accum.solve() else {
             continue;
         };
 
+        // Add fixed costs to get the full cost.
         let annual_fixed_costs_per_flow = *existing_fixed_costs_cache
             .entry(asset.clone())
             .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activities[asset]));
-
         let full_cost = avg_mc + annual_fixed_costs_per_flow;
+
+        // Take the max across assets for each group
         group_prices
             .entry((commodity_id.clone(), region_id.clone(), group.clone()))
             .and_modify(|c| *c = c.max(full_cost))
@@ -707,14 +731,17 @@ where
         }
     }
 
-    // Candidate assets: assume full dispatch, use limit weights, take the min across candidates
-    let mut cand_accum: HashMap<_, (MoneyPerFlow, Dimensionless)> = HashMap::new();
+    // Candidate assets: weight marginal costs by activity limits (i.e. assume full utilisation)
+    let mut cand_accum: HashMap<_, GroupAccum> = HashMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
+
+        // Get activity limits: used as a to weight marginal costs for seasonal/annual commodities
         let activity_limit = *asset
             .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
             .end();
 
+        // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
             upstream_prices,
             year,
@@ -736,24 +763,21 @@ where
                 region_id.clone(),
                 group,
             );
-            let (cost_sum, weight_sum) = cand_accum
-                .entry(key)
-                .or_insert((MoneyPerFlow(0.0), Dimensionless(0.0)));
-            *cost_sum += marginal_cost * Dimensionless(activity_limit.value());
-            *weight_sum += Dimensionless(activity_limit.value());
+            let accum = cand_accum.entry(key).or_default();
+            accum.add(marginal_cost, Activity(0.0), activity_limit);
         }
     }
 
     // Compute per-group weighted average, add fixed costs, take the min across candidates
     let mut cand_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
     let mut cand_group_prices: HashMap<_, MoneyPerFlow> = HashMap::new();
-    for ((asset, commodity_id, region_id, group), (cost_sum, weight_sum)) in &cand_accum {
-        if *weight_sum < Dimensionless::EPSILON {
+    for ((asset, commodity_id, region_id, group), accum) in &cand_accum {
+        // Solve the weighted average marginal cost for this group
+        let Some(avg_mc) = accum.solve() else {
             continue;
-        }
-        let avg_mc = *cost_sum / *weight_sum;
+        };
 
-        // For candidates, assume full annual dispatch
+        // Add fixed costs to get the full cost. For candidates we assume full utilisation
         let annual_fixed_costs_per_flow = *cand_fixed_costs_cache
             .entry(asset.clone())
             .or_insert_with(|| {
@@ -763,8 +787,10 @@ where
                         .end(),
                 )
             });
-
         let full_cost = avg_mc + annual_fixed_costs_per_flow;
+
+        // Take the min across candidates for each group (i.e. the single most competitive candidate
+        // if a small amount of demand was added)
         cand_group_prices
             .entry((commodity_id.clone(), region_id.clone(), group.clone()))
             .and_modify(|c| *c = c.min(full_cost))

From f34e295dbf0f28e3a5659ab28e093a4c25d75ff8 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Thu, 12 Mar 2026 15:19:59 +0000
Subject: [PATCH 03/23] Helper for expanding prices

---
 src/simulation/prices.rs | 52 ++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 32 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 99fad537c..505e2c6c8 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -374,6 +374,22 @@ impl GroupAccum {
     }
 }
 
+/// Expand a map of per-group prices to individual time slices.
+fn expand_groups_to_prices(
+    group_prices: &HashMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
+    time_slice_info: &TimeSliceInfo,
+    out: &mut HashMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>,
+) {
+    for ((commodity_id, region_id, group), &group_price) in group_prices {
+        for (ts, _) in group.iter(time_slice_info) {
+            out.insert(
+                (commodity_id.clone(), region_id.clone(), ts.clone()),
+                group_price,
+            );
+        }
+    }
+}
+
 /// Calculate marginal cost prices for a set of commodities.
 ///
 /// This pricing strategy aims to incorporate the marginal cost of commodity production into the price.
@@ -492,14 +508,7 @@ where
 
     // Expand each group to individual time slices
     let mut prices: HashMap<_, MoneyPerFlow> = HashMap::new();
-    for ((commodity_id, region_id, group), group_price) in &group_prices {
-        for (ts, _) in group.iter(time_slice_info) {
-            prices.insert(
-                (commodity_id.clone(), region_id.clone(), ts.clone()),
-                *group_price,
-            );
-        }
-    }
+    expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
 
     // Candidate assets: weight marginal costs by activity limits (i.e. assume full utilisation)
     let mut cand_accum: HashMap<_, GroupAccum> = HashMap::new();
@@ -552,14 +561,7 @@ where
     }
 
     // Expand candidate groups to individual time slices
-    for ((commodity_id, region_id, group), group_price) in &cand_group_prices {
-        for (ts, _) in group.iter(time_slice_info) {
-            prices.insert(
-                (commodity_id.clone(), region_id.clone(), ts.clone()),
-                *group_price,
-            );
-        }
-    }
+    expand_groups_to_prices(&cand_group_prices, time_slice_info, &mut prices);
 
     prices
 }
@@ -722,14 +724,7 @@ where
 
     // Expand each group to individual time slices
     let mut prices: HashMap<_, MoneyPerFlow> = HashMap::new();
-    for ((commodity_id, region_id, group), group_price) in &group_prices {
-        for (ts, _) in group.iter(time_slice_info) {
-            prices.insert(
-                (commodity_id.clone(), region_id.clone(), ts.clone()),
-                *group_price,
-            );
-        }
-    }
+    expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
 
     // Candidate assets: weight marginal costs by activity limits (i.e. assume full utilisation)
     let mut cand_accum: HashMap<_, GroupAccum> = HashMap::new();
@@ -798,14 +793,7 @@ where
     }
 
     // Expand candidate groups to individual time slices
-    for ((commodity_id, region_id, group), group_price) in &cand_group_prices {
-        for (ts, _) in group.iter(time_slice_info) {
-            prices.insert(
-                (commodity_id.clone(), region_id.clone(), ts.clone()),
-                *group_price,
-            );
-        }
-    }
+    expand_groups_to_prices(&cand_group_prices, time_slice_info, &mut prices);
 
     prices
 }

From 02263bd5367f186b1a0da044f5496e8dc3c37dd8 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Thu, 12 Mar 2026 15:28:45 +0000
Subject: [PATCH 04/23] Get clippy to pass

---
 src/simulation/optimisation.rs |  8 --------
 src/simulation/prices.rs       | 23 +++++++++--------------
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/src/simulation/optimisation.rs b/src/simulation/optimisation.rs
index e14926033..85b7e7821 100644
--- a/src/simulation/optimisation.rs
+++ b/src/simulation/optimisation.rs
@@ -263,14 +263,6 @@ impl Solution<'_> {
             .map(|((asset, time_slice), &value)| (asset, time_slice, Activity(value)))
     }
 
-    /// Iterate over the keys for activity for each existing asset
-    pub fn iter_activity_keys_for_existing(
-        &self,
-    ) -> impl Iterator<Item = (&AssetRef, &TimeSliceID)> {
-        self.iter_activity_for_existing()
-            .map(|(asset, time_slice, _activity)| (asset, time_slice))
-    }
-
     /// Activity for each candidate asset
     pub fn iter_activity_for_candidates(
         &self,
diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 505e2c6c8..328d746c8 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -91,12 +91,9 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
 
         // Add prices for full cost commodities
         if let Some(fullcost_set) = pricing_sets.get(&PricingStrategy::FullCost) {
-            let annual_activities =
-                calculate_annual_activities(solution.iter_activity_for_existing());
             let full_cost_prices = calculate_full_cost_prices(
                 solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
-                &annual_activities,
                 &result,
                 year,
                 fullcost_set,
@@ -426,7 +423,7 @@ fn expand_groups_to_prices(
 ///
 /// For commodities with seasonal/annual time slice levels, marginal costs are weighted by
 /// activity (or the maximum potential activity for candidates) to get a time slice-weighted average
-/// marginal cost for each asset, before taking the max across assets. Consequentially, the price of
+/// marginal cost for each asset, before taking the max across assets. Consequently, the price of
 /// these commodities is flat within each season/year.
 ///
 /// # Arguments
@@ -625,7 +622,7 @@ where
 ///
 /// For commodities with seasonal/annual time slice levels, costs are weighted by activity (or the
 /// maximum potential activity for candidates) to get a time slice-weighted average cost for each
-/// asset, before taking the max across assets. Consequentially, the price of these commodities is
+/// asset, before taking the max across assets. Consequently, the price of these commodities is
 /// flat within each season/year.
 ///
 /// # Arguments
@@ -644,10 +641,10 @@ where
 /// # Returns
 ///
 /// A map of full cost prices for the specified markets in all time slices
+#[allow(clippy::too_many_lines)]
 fn calculate_full_cost_prices<'a, I, J>(
     activity_for_existing: I,
     activity_keys_for_candidates: J,
-    annual_activities: &HashMap<AssetRef, Activity>,
     upstream_prices: &CommodityPrices,
     year: u32,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
@@ -658,16 +655,18 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
+    // We need annual activities for existing assets to calculate capital costs per flow
+    let activity_for_existing: Vec<_> = activity_for_existing.collect();
+    let annual_activities = calculate_annual_activities(activity_for_existing.iter().copied());
+
     // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
-    // commodities, marginal costs are weighted by activity (or the activity limit for assets with
-    // no activity)
+    // commodities, marginal costs are weighted by activity
     let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
 
-        // If annual activity is zero, we can't calculate a capital cost per flow, so skip this
-        // asset.
+        // If annual activity is zero, we can't calculate a capital cost per flow, so skip
         if annual_activity < Activity::EPSILON {
             continue;
         }
@@ -1078,13 +1077,9 @@ mod tests {
         let existing = vec![(&asset_ref, &time_slice, Activity(2.0))];
         let candidates = Vec::new();
 
-        let mut annual_activities = HashMap::new();
-        annual_activities.insert(asset_ref.clone(), Activity(2.0));
-
         let prices = calculate_full_cost_prices(
             existing.into_iter(),
             candidates.into_iter(),
-            &annual_activities,
             &shadow_prices,
             2015u32,
             &markets,

From fd2eb5846227891c06ed48d35f8aed71e65a9f8b Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Thu, 12 Mar 2026 15:35:47 +0000
Subject: [PATCH 05/23] Add tests for GroupAccum

---
 src/simulation/prices.rs | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 328d746c8..4c6efd3e7 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -887,6 +887,45 @@ mod tests {
         assert!((p - expected).abs() < MoneyPerFlow::EPSILON);
     }
 
+    #[test]
+    fn group_accum_empty_returns_none() {
+        assert!(GroupAccum::default().solve().is_none());
+    }
+
+    #[test]
+    fn group_accum_returns_none_when_both_denominators_zero() {
+        let mut accum = GroupAccum::default();
+        accum.add(MoneyPerFlow(10.0), Activity(0.0), Activity(0.0));
+        assert!(accum.solve().is_none());
+    }
+
+    #[test]
+    fn group_accum_uses_activity_weight() {
+        let mut accum = GroupAccum::default();
+        // Two entries: cost 10 with activity 1, cost 20 with activity 3 → weighted avg = 17.5
+        accum.add(MoneyPerFlow(10.0), Activity(1.0), Activity(1.0));
+        accum.add(MoneyPerFlow(20.0), Activity(3.0), Activity(3.0));
+        let result = accum.solve().unwrap();
+        assert!((result - MoneyPerFlow(17.5)).abs() < MoneyPerFlow::EPSILON);
+    }
+
+    #[test]
+    fn group_accum_single_entry_returns_same_cost() {
+        let mut accum = GroupAccum::default();
+        accum.add(MoneyPerFlow(42.0), Activity(5.0), Activity(5.0));
+        assert_eq!(accum.solve().unwrap(), MoneyPerFlow(42.0));
+    }
+
+    #[test]
+    fn group_accum_falls_back_to_activity_limit_when_no_activity() {
+        let mut accum = GroupAccum::default();
+        // Zero activity, but non-zero activity limits → should use backup weights
+        accum.add(MoneyPerFlow(10.0), Activity(0.0), Activity(1.0));
+        accum.add(MoneyPerFlow(20.0), Activity(0.0), Activity(3.0));
+        let result = accum.solve().unwrap();
+        assert!((result - MoneyPerFlow(17.5)).abs() < MoneyPerFlow::EPSILON);
+    }
+
     #[rstest]
     #[case(MoneyPerFlow(100.0), MoneyPerFlow(100.0), Dimensionless(0.0), true)] // exactly equal
     #[case(MoneyPerFlow(100.0), MoneyPerFlow(105.0), Dimensionless(0.1), true)] // within tolerance

From 2b6df894201c2dd752ca7335cd6461544f8542c4 Mon Sep 17 00:00:00 2001
From: Tom Bland <t.bland@imperial.ac.uk>
Date: Thu, 12 Mar 2026 16:53:56 +0000
Subject: [PATCH 06/23] Update src/simulation/prices.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---
 src/simulation/prices.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 4c6efd3e7..3aeb5c85d 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -630,8 +630,6 @@ where
 /// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
 ///   solution for existing assets
 /// * `activity_keys_for_candidates` - Iterator over `(asset, time_slice)` for candidate assets
-/// * `annual_activities` - Map of annual activities for each asset computed by
-///   `calculate_annual_activities`. This only needs to include existing assets.
 /// * `upstream_prices` - Prices for commodities upstream of the ones we are calculating prices for
 /// * `year` - The year for which prices are being calculated
 /// * `markets_to_price` - Set of markets to calculate full cost prices for

From ae59c1d9f7e11cc81e48568458de8bc0aa1fa1f8 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Fri, 13 Mar 2026 15:49:03 +0000
Subject: [PATCH 07/23] Revert "Get clippy to pass"

This reverts commit 02263bd5367f186b1a0da044f5496e8dc3c37dd8.
---
 src/simulation/optimisation.rs |  8 ++++++++
 src/simulation/prices.rs       | 23 ++++++++++++++---------
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/src/simulation/optimisation.rs b/src/simulation/optimisation.rs
index 85b7e7821..e14926033 100644
--- a/src/simulation/optimisation.rs
+++ b/src/simulation/optimisation.rs
@@ -263,6 +263,14 @@ impl Solution<'_> {
             .map(|((asset, time_slice), &value)| (asset, time_slice, Activity(value)))
     }
 
+    /// Iterate over the keys for activity for each existing asset
+    pub fn iter_activity_keys_for_existing(
+        &self,
+    ) -> impl Iterator<Item = (&AssetRef, &TimeSliceID)> {
+        self.iter_activity_for_existing()
+            .map(|(asset, time_slice, _activity)| (asset, time_slice))
+    }
+
     /// Activity for each candidate asset
     pub fn iter_activity_for_candidates(
         &self,
diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 3aeb5c85d..b61ec895e 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -91,9 +91,12 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
 
         // Add prices for full cost commodities
         if let Some(fullcost_set) = pricing_sets.get(&PricingStrategy::FullCost) {
+            let annual_activities =
+                calculate_annual_activities(solution.iter_activity_for_existing());
             let full_cost_prices = calculate_full_cost_prices(
                 solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
+                &annual_activities,
                 &result,
                 year,
                 fullcost_set,
@@ -423,7 +426,7 @@ fn expand_groups_to_prices(
 ///
 /// For commodities with seasonal/annual time slice levels, marginal costs are weighted by
 /// activity (or the maximum potential activity for candidates) to get a time slice-weighted average
-/// marginal cost for each asset, before taking the max across assets. Consequently, the price of
+/// marginal cost for each asset, before taking the max across assets. Consequentially, the price of
 /// these commodities is flat within each season/year.
 ///
 /// # Arguments
@@ -622,7 +625,7 @@ where
 ///
 /// For commodities with seasonal/annual time slice levels, costs are weighted by activity (or the
 /// maximum potential activity for candidates) to get a time slice-weighted average cost for each
-/// asset, before taking the max across assets. Consequently, the price of these commodities is
+/// asset, before taking the max across assets. Consequentially, the price of these commodities is
 /// flat within each season/year.
 ///
 /// # Arguments
@@ -639,10 +642,10 @@ where
 /// # Returns
 ///
 /// A map of full cost prices for the specified markets in all time slices
-#[allow(clippy::too_many_lines)]
 fn calculate_full_cost_prices<'a, I, J>(
     activity_for_existing: I,
     activity_keys_for_candidates: J,
+    annual_activities: &HashMap<AssetRef, Activity>,
     upstream_prices: &CommodityPrices,
     year: u32,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
@@ -653,18 +656,16 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // We need annual activities for existing assets to calculate capital costs per flow
-    let activity_for_existing: Vec<_> = activity_for_existing.collect();
-    let annual_activities = calculate_annual_activities(activity_for_existing.iter().copied());
-
     // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
-    // commodities, marginal costs are weighted by activity
+    // commodities, marginal costs are weighted by activity (or the activity limit for assets with
+    // no activity)
     let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
 
-        // If annual activity is zero, we can't calculate a capital cost per flow, so skip
+        // If annual activity is zero, we can't calculate a capital cost per flow, so skip this
+        // asset.
         if annual_activity < Activity::EPSILON {
             continue;
         }
@@ -1114,9 +1115,13 @@ mod tests {
         let existing = vec![(&asset_ref, &time_slice, Activity(2.0))];
         let candidates = Vec::new();
 
+        let mut annual_activities = HashMap::new();
+        annual_activities.insert(asset_ref.clone(), Activity(2.0));
+
         let prices = calculate_full_cost_prices(
             existing.into_iter(),
             candidates.into_iter(),
+            &annual_activities,
             &shadow_prices,
             2015u32,
             &markets,

From 6b5f51abcad825217cda11391b44f2b5a971f9d5 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Fri, 13 Mar 2026 15:59:00 +0000
Subject: [PATCH 08/23] Avoid repeatedly calculating annual activities

---
 src/simulation/optimisation.rs |  8 --------
 src/simulation/prices.rs       | 15 ++++++++++-----
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/simulation/optimisation.rs b/src/simulation/optimisation.rs
index e14926033..85b7e7821 100644
--- a/src/simulation/optimisation.rs
+++ b/src/simulation/optimisation.rs
@@ -263,14 +263,6 @@ impl Solution<'_> {
             .map(|((asset, time_slice), &value)| (asset, time_slice, Activity(value)))
     }
 
-    /// Iterate over the keys for activity for each existing asset
-    pub fn iter_activity_keys_for_existing(
-        &self,
-    ) -> impl Iterator<Item = (&AssetRef, &TimeSliceID)> {
-        self.iter_activity_for_existing()
-            .map(|(asset, time_slice, _activity)| (asset, time_slice))
-    }
-
     /// Activity for each candidate asset
     pub fn iter_activity_for_candidates(
         &self,
diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index b61ec895e..b45e10955 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -32,6 +32,9 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
     // Set up empty prices map
     let mut result = CommodityPrices::default();
 
+    // Lazily computed only if at least one FullCost market is encountered.
+    let mut annual_activities: Option<HashMap<AssetRef, Activity>> = None;
+
     // Get investment order for the year - prices will be calculated in the reverse of this order
     let investment_order = &model.investment_order[&year];
 
@@ -91,12 +94,13 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
 
         // Add prices for full cost commodities
         if let Some(fullcost_set) = pricing_sets.get(&PricingStrategy::FullCost) {
-            let annual_activities =
-                calculate_annual_activities(solution.iter_activity_for_existing());
+            let annual_activities = annual_activities.get_or_insert_with(|| {
+                calculate_annual_activities(solution.iter_activity_for_existing())
+            });
             let full_cost_prices = calculate_full_cost_prices(
                 solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
-                &annual_activities,
+                annual_activities,
                 &result,
                 year,
                 fullcost_set,
@@ -426,7 +430,7 @@ fn expand_groups_to_prices(
 ///
 /// For commodities with seasonal/annual time slice levels, marginal costs are weighted by
 /// activity (or the maximum potential activity for candidates) to get a time slice-weighted average
-/// marginal cost for each asset, before taking the max across assets. Consequentially, the price of
+/// marginal cost for each asset, before taking the max across assets. Consequently, the price of
 /// these commodities is flat within each season/year.
 ///
 /// # Arguments
@@ -625,7 +629,7 @@ where
 ///
 /// For commodities with seasonal/annual time slice levels, costs are weighted by activity (or the
 /// maximum potential activity for candidates) to get a time slice-weighted average cost for each
-/// asset, before taking the max across assets. Consequentially, the price of these commodities is
+/// asset, before taking the max across assets. Consequently, the price of these commodities is
 /// flat within each season/year.
 ///
 /// # Arguments
@@ -642,6 +646,7 @@ where
 /// # Returns
 ///
 /// A map of full cost prices for the specified markets in all time slices
+#[allow(clippy::too_many_arguments)]
 fn calculate_full_cost_prices<'a, I, J>(
     activity_for_existing: I,
     activity_keys_for_candidates: J,

From e2fdb034f3346d4055f150c020774c650714b58c Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Mon, 16 Mar 2026 17:16:00 +0000
Subject: [PATCH 09/23] Better use of indexmaps

---
 src/simulation/prices.rs | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 0c5d1046d..bcf6571bf 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -463,7 +463,7 @@ where
     // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
     // commodities, marginal costs are weighted by activity (or the activity limit for assets with
     // no activity)
-    let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
+    let mut existing_accum: IndexMap<_, GroupAccum> = IndexMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
 
@@ -514,12 +514,12 @@ where
     let mut prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
     expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
 
-    // Candidate assets: weight marginal costs by activity limits (i.e. assume full utilisation)
-    let mut cand_accum: HashMap<_, GroupAccum> = HashMap::new();
+    // Candidate assets (assume full utilisation)
+    let mut cand_accum: IndexMap<_, GroupAccum> = IndexMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
-        // Get activity limits: used as a to weight marginal costs for seasonal/annual commodities
+        // Get activity limits: used to weight marginal costs for seasonal/annual commodities
         let activity_limit = *asset
             .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
             .end();
@@ -664,7 +664,7 @@ where
     // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
     // commodities, marginal costs are weighted by activity (or the activity limit for assets with
     // no activity)
-    let mut existing_accum: HashMap<_, GroupAccum> = HashMap::new();
+    let mut existing_accum: IndexMap<_, GroupAccum> = IndexMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
@@ -729,12 +729,12 @@ where
     let mut prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
     expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
 
-    // Candidate assets: weight marginal costs by activity limits (i.e. assume full utilisation)
-    let mut cand_accum: HashMap<_, GroupAccum> = HashMap::new();
+    // Candidate assets (assume full utilisation)
+    let mut cand_accum: IndexMap<_, GroupAccum> = IndexMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
-        // Get activity limits: used as a to weight marginal costs for seasonal/annual commodities
+        // Get activity limits: used to weight marginal costs for seasonal/annual commodities
         let activity_limit = *asset
             .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
             .end();

From 0be85fd9c11b3282379d0aee41c6baf6ef6f6709 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Mon, 16 Mar 2026 18:12:03 +0000
Subject: [PATCH 10/23] Add generic accumulator trait

---
 src/simulation.rs             |   1 +
 src/simulation/accumulator.rs | 186 ++++++++++++++++++++++++++++++++++
 src/simulation/prices.rs      | 129 ++++++-----------------
 3 files changed, 216 insertions(+), 100 deletions(-)
 create mode 100644 src/simulation/accumulator.rs

diff --git a/src/simulation.rs b/src/simulation.rs
index 20d4d70c5..50a3725db 100644
--- a/src/simulation.rs
+++ b/src/simulation.rs
@@ -14,6 +14,7 @@ pub mod optimisation;
 use optimisation::{DispatchRun, FlowMap};
 pub mod investment;
 use investment::perform_agent_investment;
+pub mod accumulator;
 pub mod prices;
 pub use prices::CommodityPrices;
 
diff --git a/src/simulation/accumulator.rs b/src/simulation/accumulator.rs
new file mode 100644
index 000000000..1e751d357
--- /dev/null
+++ b/src/simulation/accumulator.rs
@@ -0,0 +1,186 @@
+//! Generic accumulator primitives for aggregation.
+use crate::units::{Dimensionless, UnitType};
+
+/// Generic accumulator contract.
+pub trait Accumulator<Input> {
+    /// The type yielded when the accumulator is solved.
+    type Output;
+
+    /// Add a single input sample.
+    fn add(&mut self, input: Input);
+
+    /// Finalise the accumulator and return a result if enough information was provided.
+    fn finalise(&self) -> Option<Self::Output>;
+}
+
+/// Maximum value accumulator.
+#[derive(Clone, Copy, Debug, Default)]
+pub struct MaxAccumulator<T>
+where
+    T: UnitType,
+{
+    value: Option<T>,
+}
+
+impl<T> Accumulator<T> for MaxAccumulator<T>
+where
+    T: UnitType,
+{
+    type Output = T;
+
+    fn add(&mut self, input: T) {
+        self.value = Some(match self.value {
+            Some(current) => current.max(input),
+            None => input,
+        });
+    }
+
+    fn finalise(&self) -> Option<Self::Output> {
+        self.value
+    }
+}
+
+/// Minimum value accumulator.
+#[derive(Clone, Copy, Debug, Default)]
+pub struct MinAccumulator<T>
+where
+    T: UnitType,
+{
+    value: Option<T>,
+}
+
+impl<T> Accumulator<T> for MinAccumulator<T>
+where
+    T: UnitType,
+{
+    type Output = T;
+
+    fn add(&mut self, input: T) {
+        self.value = Some(match self.value {
+            Some(current) => current.min(input),
+            None => input,
+        });
+    }
+
+    fn finalise(&self) -> Option<Self::Output> {
+        self.value
+    }
+}
+
+/// Weighted average accumulator.
+#[derive(Clone, Copy, Debug)]
+pub struct WeightedAverageAccumulator<T>
+where
+    T: UnitType,
+{
+    numerator: T,
+    denominator: Dimensionless,
+}
+
+impl<T> Default for WeightedAverageAccumulator<T>
+where
+    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
+{
+    fn default() -> Self {
+        Self {
+            numerator: T::new(0.0),
+            denominator: Dimensionless(0.0),
+        }
+    }
+}
+
+impl<T> WeightedAverageAccumulator<T>
+where
+    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
+{
+    /// Add a weighted value.
+    pub fn add(&mut self, value: T, weight: Dimensionless) {
+        self.numerator += value * weight;
+        self.denominator += weight;
+    }
+
+    /// Solve the weighted average.
+    pub fn finalise(&self) -> Option<T> {
+        (self.denominator > Dimensionless::EPSILON).then(|| self.numerator / self.denominator)
+    }
+}
+
+impl<T> Accumulator<(T, Dimensionless)> for WeightedAverageAccumulator<T>
+where
+    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
+{
+    type Output = T;
+
+    fn add(&mut self, input: (T, Dimensionless)) {
+        self.add(input.0, input.1);
+    }
+
+    fn finalise(&self) -> Option<Self::Output> {
+        Self::finalise(self)
+    }
+}
+
+/// Weighted average accumulator with a backup weighting path.
+#[derive(Clone, Copy, Debug)]
+pub struct WeightedAverageBackupAccumulator<T>
+where
+    T: UnitType,
+{
+    primary_numerator: T,
+    primary_denominator: Dimensionless,
+    backup_numerator: T,
+    backup_denominator: Dimensionless,
+}
+
+impl<T> Default for WeightedAverageBackupAccumulator<T>
+where
+    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
+{
+    fn default() -> Self {
+        Self {
+            primary_numerator: T::new(0.0),
+            primary_denominator: Dimensionless(0.0),
+            backup_numerator: T::new(0.0),
+            backup_denominator: Dimensionless(0.0),
+        }
+    }
+}
+
+impl<T> WeightedAverageBackupAccumulator<T>
+where
+    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
+{
+    /// Add a weighted value with a backup weight.
+    pub fn add(&mut self, value: T, weight: Dimensionless, backup_weight: Dimensionless) {
+        self.primary_numerator += value * weight;
+        self.primary_denominator += weight;
+        self.backup_numerator += value * backup_weight;
+        self.backup_denominator += backup_weight;
+    }
+
+    /// Solve the weighted average, falling back to backup weights if needed.
+    pub fn finalise(&self) -> Option<T> {
+        if self.primary_denominator > Dimensionless::EPSILON {
+            Some(self.primary_numerator / self.primary_denominator)
+        } else if self.backup_denominator > Dimensionless::EPSILON {
+            Some(self.backup_numerator / self.backup_denominator)
+        } else {
+            None
+        }
+    }
+}
+
+impl<T> Accumulator<(T, Dimensionless, Dimensionless)> for WeightedAverageBackupAccumulator<T>
+where
+    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
+{
+    type Output = T;
+
+    fn add(&mut self, input: (T, Dimensionless, Dimensionless)) {
+        self.add(input.0, input.1, input.2);
+    }
+
+    fn finalise(&self) -> Option<Self::Output> {
+        Self::finalise(self)
+    }
+}
diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index bcf6571bf..d84e3baad 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -3,6 +3,9 @@ use crate::asset::AssetRef;
 use crate::commodity::{CommodityID, CommodityMap, PricingStrategy};
 use crate::model::Model;
 use crate::region::RegionID;
+use crate::simulation::accumulator::{
+    WeightedAverageAccumulator, WeightedAverageBackupAccumulator,
+};
 use crate::simulation::optimisation::Solution;
 use crate::time_slice::{TimeSliceID, TimeSliceInfo, TimeSliceSelection};
 use crate::units::{Activity, Dimensionless, MoneyPerActivity, MoneyPerFlow, Year};
@@ -330,54 +333,6 @@ where
     scarcity_prices
 }
 
-/// Helper struct for accumulating weighted marginal costs for a group of (asset, commodity, region,
-/// time slice selection) tuples when calculating marginal cost or full cost prices.
-///
-/// For seasonal/annual commodities, marginal costs are weighted by activity (or the activity limit
-/// for assets with no activity) to get a time slice-weighted average marginal cost for the group.
-struct GroupAccum {
-    // Sum of (marginal cost * activity) across all tuples in the group
-    weighted_cost_numerator: MoneyPerFlow,
-    // Sum of activity across all tuples in the group
-    weighted_cost_denominator: Dimensionless,
-    // Backup numerator and denominator for the case where there's no activity across the group
-    backup_numerator: MoneyPerFlow,
-    backup_denominator: Dimensionless,
-}
-
-impl Default for GroupAccum {
-    fn default() -> Self {
-        Self {
-            weighted_cost_numerator: MoneyPerFlow(0.0),
-            weighted_cost_denominator: Dimensionless(0.0),
-            backup_numerator: MoneyPerFlow(0.0),
-            backup_denominator: Dimensionless(0.0),
-        }
-    }
-}
-
-impl GroupAccum {
-    /// Add a marginal cost to the accumulator
-    fn add(&mut self, marginal_cost: MoneyPerFlow, activity: Activity, activity_limit: Activity) {
-        self.weighted_cost_numerator += marginal_cost * Dimensionless(activity.value());
-        self.weighted_cost_denominator += Dimensionless(activity.value());
-        self.backup_numerator += marginal_cost * Dimensionless(activity_limit.value());
-        self.backup_denominator += Dimensionless(activity_limit.value());
-    }
-
-    /// Solve the weighted average marginal cost for the group, using the backup weights (activity
-    /// limits) if there's no activity across the group. If both denominators are zero, return None.
-    fn solve(&self) -> Option<MoneyPerFlow> {
-        if self.weighted_cost_denominator > Dimensionless::EPSILON {
-            Some(self.weighted_cost_numerator / self.weighted_cost_denominator)
-        } else if self.backup_denominator > Dimensionless::EPSILON {
-            Some(self.backup_numerator / self.backup_denominator)
-        } else {
-            None
-        }
-    }
-}
-
 /// Expand a map of per-group prices to individual time slices.
 fn expand_groups_to_prices(
     group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
@@ -463,7 +418,8 @@ where
     // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
     // commodities, marginal costs are weighted by activity (or the activity limit for assets with
     // no activity)
-    let mut existing_accum: IndexMap<_, GroupAccum> = IndexMap::new();
+    let mut existing_accum: IndexMap<_, WeightedAverageBackupAccumulator<MoneyPerFlow>> =
+        IndexMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
 
@@ -479,9 +435,12 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
+            // Get the group according to the commodity's time slice level
             let group = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
+
+            // Insert the marginal cost into the accumulator for this group
             let key = (
                 asset.clone(),
                 commodity_id.clone(),
@@ -489,7 +448,11 @@ where
                 group,
             );
             let accum = existing_accum.entry(key).or_default();
-            accum.add(marginal_cost, activity, activity_limit);
+            accum.add(
+                marginal_cost,
+                Dimensionless(activity.value()),
+                Dimensionless(activity_limit.value()),
+            );
         }
     }
 
@@ -498,7 +461,7 @@ where
     let mut priced_groups: HashSet<_> = HashSet::new();
     for ((_, commodity_id, region_id, group), accum) in &existing_accum {
         // Solve the weighted average marginal cost for this group
-        let Some(avg_cost) = accum.solve() else {
+        let Some(avg_cost) = accum.finalise() else {
             continue;
         };
 
@@ -515,7 +478,7 @@ where
     expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
 
     // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, GroupAccum> = IndexMap::new();
+    let mut cand_accum: IndexMap<_, WeightedAverageAccumulator<MoneyPerFlow>> = IndexMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
@@ -547,7 +510,7 @@ where
                 group,
             );
             let accum = cand_accum.entry(key).or_default();
-            accum.add(marginal_cost, Activity(0.0), activity_limit);
+            accum.add(marginal_cost, Dimensionless(activity_limit.value()));
         }
     }
 
@@ -555,7 +518,7 @@ where
     // (i.e. the single most competitive candidate if a small amount of demand was added)
     let mut cand_group_prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
     for ((_, commodity_id, region_id, group), accum) in &cand_accum {
-        let Some(avg_cost) = accum.solve() else {
+        let Some(avg_cost) = accum.finalise() else {
             continue;
         };
         cand_group_prices
@@ -646,7 +609,7 @@ where
 /// # Returns
 ///
 /// A map of full cost prices for the specified markets in all time slices
-#[allow(clippy::too_many_arguments)]
+#[allow(clippy::too_many_arguments, clippy::too_many_lines)]
 fn calculate_full_cost_prices<'a, I, J>(
     activity_for_existing: I,
     activity_keys_for_candidates: J,
@@ -664,7 +627,8 @@ where
     // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
     // commodities, marginal costs are weighted by activity (or the activity limit for assets with
     // no activity)
-    let mut existing_accum: IndexMap<_, GroupAccum> = IndexMap::new();
+    let mut existing_accum: IndexMap<_, WeightedAverageBackupAccumulator<MoneyPerFlow>> =
+        IndexMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
@@ -697,7 +661,11 @@ where
                 group,
             );
             let accum = existing_accum.entry(key).or_default();
-            accum.add(marginal_cost, activity, activity_limit);
+            accum.add(
+                marginal_cost,
+                Dimensionless(activity.value()),
+                Dimensionless(activity_limit.value()),
+            );
         }
     }
 
@@ -707,7 +675,7 @@ where
     let mut existing_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
     for ((asset, commodity_id, region_id, group), accum) in &existing_accum {
         // Solve the weighted average marginal cost for this group
-        let Some(avg_mc) = accum.solve() else {
+        let Some(avg_mc) = accum.finalise() else {
             continue;
         };
 
@@ -730,7 +698,7 @@ where
     expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
 
     // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, GroupAccum> = IndexMap::new();
+    let mut cand_accum: IndexMap<_, WeightedAverageAccumulator<MoneyPerFlow>> = IndexMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
@@ -762,7 +730,7 @@ where
                 group,
             );
             let accum = cand_accum.entry(key).or_default();
-            accum.add(marginal_cost, Activity(0.0), activity_limit);
+            accum.add(marginal_cost, Dimensionless(activity_limit.value()));
         }
     }
 
@@ -771,7 +739,7 @@ where
     let mut cand_group_prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
     for ((asset, commodity_id, region_id, group), accum) in &cand_accum {
         // Solve the weighted average marginal cost for this group
-        let Some(avg_mc) = accum.solve() else {
+        let Some(avg_mc) = accum.finalise() else {
             continue;
         };
 
@@ -891,45 +859,6 @@ mod tests {
         assert!((p - expected).abs() < MoneyPerFlow::EPSILON);
     }
 
-    #[test]
-    fn group_accum_empty_returns_none() {
-        assert!(GroupAccum::default().solve().is_none());
-    }
-
-    #[test]
-    fn group_accum_returns_none_when_both_denominators_zero() {
-        let mut accum = GroupAccum::default();
-        accum.add(MoneyPerFlow(10.0), Activity(0.0), Activity(0.0));
-        assert!(accum.solve().is_none());
-    }
-
-    #[test]
-    fn group_accum_uses_activity_weight() {
-        let mut accum = GroupAccum::default();
-        // Two entries: cost 10 with activity 1, cost 20 with activity 3 → weighted avg = 17.5
-        accum.add(MoneyPerFlow(10.0), Activity(1.0), Activity(1.0));
-        accum.add(MoneyPerFlow(20.0), Activity(3.0), Activity(3.0));
-        let result = accum.solve().unwrap();
-        assert!((result - MoneyPerFlow(17.5)).abs() < MoneyPerFlow::EPSILON);
-    }
-
-    #[test]
-    fn group_accum_single_entry_returns_same_cost() {
-        let mut accum = GroupAccum::default();
-        accum.add(MoneyPerFlow(42.0), Activity(5.0), Activity(5.0));
-        assert_eq!(accum.solve().unwrap(), MoneyPerFlow(42.0));
-    }
-
-    #[test]
-    fn group_accum_falls_back_to_activity_limit_when_no_activity() {
-        let mut accum = GroupAccum::default();
-        // Zero activity, but non-zero activity limits → should use backup weights
-        accum.add(MoneyPerFlow(10.0), Activity(0.0), Activity(1.0));
-        accum.add(MoneyPerFlow(20.0), Activity(0.0), Activity(3.0));
-        let result = accum.solve().unwrap();
-        assert!((result - MoneyPerFlow(17.5)).abs() < MoneyPerFlow::EPSILON);
-    }
-
     #[rstest]
     #[case(MoneyPerFlow(100.0), MoneyPerFlow(100.0), Dimensionless(0.0), true)] // exactly equal
     #[case(MoneyPerFlow(100.0), MoneyPerFlow(105.0), Dimensionless(0.1), true)] // within tolerance

From 997156272051f7b50a245a02faffc4ab605ca113 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Mon, 16 Mar 2026 20:11:01 +0000
Subject: [PATCH 11/23] Tidy up and simplify

---
 src/simulation.rs             |   1 -
 src/simulation/accumulator.rs | 186 ------------------
 src/simulation/prices.rs      | 353 +++++++++++++++++++---------------
 3 files changed, 203 insertions(+), 337 deletions(-)
 delete mode 100644 src/simulation/accumulator.rs

diff --git a/src/simulation.rs b/src/simulation.rs
index 50a3725db..20d4d70c5 100644
--- a/src/simulation.rs
+++ b/src/simulation.rs
@@ -14,7 +14,6 @@ pub mod optimisation;
 use optimisation::{DispatchRun, FlowMap};
 pub mod investment;
 use investment::perform_agent_investment;
-pub mod accumulator;
 pub mod prices;
 pub use prices::CommodityPrices;
 
diff --git a/src/simulation/accumulator.rs b/src/simulation/accumulator.rs
deleted file mode 100644
index 1e751d357..000000000
--- a/src/simulation/accumulator.rs
+++ /dev/null
@@ -1,186 +0,0 @@
-//! Generic accumulator primitives for aggregation.
-use crate::units::{Dimensionless, UnitType};
-
-/// Generic accumulator contract.
-pub trait Accumulator<Input> {
-    /// The type yielded when the accumulator is solved.
-    type Output;
-
-    /// Add a single input sample.
-    fn add(&mut self, input: Input);
-
-    /// Finalise the accumulator and return a result if enough information was provided.
-    fn finalise(&self) -> Option<Self::Output>;
-}
-
-/// Maximum value accumulator.
-#[derive(Clone, Copy, Debug, Default)]
-pub struct MaxAccumulator<T>
-where
-    T: UnitType,
-{
-    value: Option<T>,
-}
-
-impl<T> Accumulator<T> for MaxAccumulator<T>
-where
-    T: UnitType,
-{
-    type Output = T;
-
-    fn add(&mut self, input: T) {
-        self.value = Some(match self.value {
-            Some(current) => current.max(input),
-            None => input,
-        });
-    }
-
-    fn finalise(&self) -> Option<Self::Output> {
-        self.value
-    }
-}
-
-/// Minimum value accumulator.
-#[derive(Clone, Copy, Debug, Default)]
-pub struct MinAccumulator<T>
-where
-    T: UnitType,
-{
-    value: Option<T>,
-}
-
-impl<T> Accumulator<T> for MinAccumulator<T>
-where
-    T: UnitType,
-{
-    type Output = T;
-
-    fn add(&mut self, input: T) {
-        self.value = Some(match self.value {
-            Some(current) => current.min(input),
-            None => input,
-        });
-    }
-
-    fn finalise(&self) -> Option<Self::Output> {
-        self.value
-    }
-}
-
-/// Weighted average accumulator.
-#[derive(Clone, Copy, Debug)]
-pub struct WeightedAverageAccumulator<T>
-where
-    T: UnitType,
-{
-    numerator: T,
-    denominator: Dimensionless,
-}
-
-impl<T> Default for WeightedAverageAccumulator<T>
-where
-    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
-{
-    fn default() -> Self {
-        Self {
-            numerator: T::new(0.0),
-            denominator: Dimensionless(0.0),
-        }
-    }
-}
-
-impl<T> WeightedAverageAccumulator<T>
-where
-    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
-{
-    /// Add a weighted value.
-    pub fn add(&mut self, value: T, weight: Dimensionless) {
-        self.numerator += value * weight;
-        self.denominator += weight;
-    }
-
-    /// Solve the weighted average.
-    pub fn finalise(&self) -> Option<T> {
-        (self.denominator > Dimensionless::EPSILON).then(|| self.numerator / self.denominator)
-    }
-}
-
-impl<T> Accumulator<(T, Dimensionless)> for WeightedAverageAccumulator<T>
-where
-    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
-{
-    type Output = T;
-
-    fn add(&mut self, input: (T, Dimensionless)) {
-        self.add(input.0, input.1);
-    }
-
-    fn finalise(&self) -> Option<Self::Output> {
-        Self::finalise(self)
-    }
-}
-
-/// Weighted average accumulator with a backup weighting path.
-#[derive(Clone, Copy, Debug)]
-pub struct WeightedAverageBackupAccumulator<T>
-where
-    T: UnitType,
-{
-    primary_numerator: T,
-    primary_denominator: Dimensionless,
-    backup_numerator: T,
-    backup_denominator: Dimensionless,
-}
-
-impl<T> Default for WeightedAverageBackupAccumulator<T>
-where
-    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
-{
-    fn default() -> Self {
-        Self {
-            primary_numerator: T::new(0.0),
-            primary_denominator: Dimensionless(0.0),
-            backup_numerator: T::new(0.0),
-            backup_denominator: Dimensionless(0.0),
-        }
-    }
-}
-
-impl<T> WeightedAverageBackupAccumulator<T>
-where
-    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
-{
-    /// Add a weighted value with a backup weight.
-    pub fn add(&mut self, value: T, weight: Dimensionless, backup_weight: Dimensionless) {
-        self.primary_numerator += value * weight;
-        self.primary_denominator += weight;
-        self.backup_numerator += value * backup_weight;
-        self.backup_denominator += backup_weight;
-    }
-
-    /// Solve the weighted average, falling back to backup weights if needed.
-    pub fn finalise(&self) -> Option<T> {
-        if self.primary_denominator > Dimensionless::EPSILON {
-            Some(self.primary_numerator / self.primary_denominator)
-        } else if self.backup_denominator > Dimensionless::EPSILON {
-            Some(self.backup_numerator / self.backup_denominator)
-        } else {
-            None
-        }
-    }
-}
-
-impl<T> Accumulator<(T, Dimensionless, Dimensionless)> for WeightedAverageBackupAccumulator<T>
-where
-    T: UnitType + std::ops::Div<Dimensionless, Output = T>,
-{
-    type Output = T;
-
-    fn add(&mut self, input: (T, Dimensionless, Dimensionless)) {
-        self.add(input.0, input.1, input.2);
-    }
-
-    fn finalise(&self) -> Option<Self::Output> {
-        Self::finalise(self)
-    }
-}
diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index d84e3baad..3f4eb7b92 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -3,9 +3,6 @@ use crate::asset::AssetRef;
 use crate::commodity::{CommodityID, CommodityMap, PricingStrategy};
 use crate::model::Model;
 use crate::region::RegionID;
-use crate::simulation::accumulator::{
-    WeightedAverageAccumulator, WeightedAverageBackupAccumulator,
-};
 use crate::simulation::optimisation::Solution;
 use crate::time_slice::{TimeSliceID, TimeSliceInfo, TimeSliceSelection};
 use crate::units::{Activity, Dimensionless, MoneyPerActivity, MoneyPerFlow, Year};
@@ -13,6 +10,76 @@ use anyhow::Result;
 use indexmap::IndexMap;
 use std::collections::{HashMap, HashSet};
 
+/// Weighted average accumulator for `MoneyPerFlow`.
+#[derive(Clone, Copy, Debug)]
+struct WeightedAverageAccumulator {
+    numerator: MoneyPerFlow,
+    denominator: Dimensionless,
+}
+
+impl Default for WeightedAverageAccumulator {
+    fn default() -> Self {
+        Self {
+            numerator: MoneyPerFlow(0.0),
+            denominator: Dimensionless(0.0),
+        }
+    }
+}
+
+impl WeightedAverageAccumulator {
+    /// Add a weighted value.
+    fn add(&mut self, value: MoneyPerFlow, weight: Dimensionless) {
+        self.numerator += value * weight;
+        self.denominator += weight;
+    }
+
+    /// Solve the weighted average.
+    fn finalise(&self) -> Option<MoneyPerFlow> {
+        (self.denominator > Dimensionless::EPSILON).then(|| self.numerator / self.denominator)
+    }
+}
+
+/// Weighted average accumulator with a backup weighting path for `MoneyPerFlow`.
+#[derive(Clone, Copy, Debug)]
+struct WeightedAverageBackupAccumulator {
+    primary_numerator: MoneyPerFlow,
+    primary_denominator: Dimensionless,
+    backup_numerator: MoneyPerFlow,
+    backup_denominator: Dimensionless,
+}
+
+impl Default for WeightedAverageBackupAccumulator {
+    fn default() -> Self {
+        Self {
+            primary_numerator: MoneyPerFlow(0.0),
+            primary_denominator: Dimensionless(0.0),
+            backup_numerator: MoneyPerFlow(0.0),
+            backup_denominator: Dimensionless(0.0),
+        }
+    }
+}
+
+impl WeightedAverageBackupAccumulator {
+    /// Add a weighted value with a backup weight.
+    fn add(&mut self, value: MoneyPerFlow, weight: Dimensionless, backup_weight: Dimensionless) {
+        self.primary_numerator += value * weight;
+        self.primary_denominator += weight;
+        self.backup_numerator += value * backup_weight;
+        self.backup_denominator += backup_weight;
+    }
+
+    /// Solve the weighted average, falling back to backup weights if needed.
+    fn finalise(&self) -> Option<MoneyPerFlow> {
+        if self.primary_denominator > Dimensionless::EPSILON {
+            Some(self.primary_numerator / self.primary_denominator)
+        } else if self.backup_denominator > Dimensionless::EPSILON {
+            Some(self.backup_numerator / self.backup_denominator)
+        } else {
+            None
+        }
+    }
+}
+
 /// Calculate commodity prices.
 ///
 /// Calculate prices for each commodity/region/time-slice according to the commodity's configured
@@ -337,16 +404,18 @@ where
 fn expand_groups_to_prices(
     group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
     time_slice_info: &TimeSliceInfo,
-    out: &mut IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>,
-) {
-    for ((commodity_id, region_id, group), &group_price) in group_prices {
-        for (ts, _) in group.iter(time_slice_info) {
-            out.insert(
-                (commodity_id.clone(), region_id.clone(), ts.clone()),
-                group_price,
-            );
-        }
-    }
+) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow> {
+    group_prices
+        .iter()
+        .flat_map(|((commodity_id, region_id, group), &group_price)| {
+            group.iter(time_slice_info).map(move |(ts, _)| {
+                (
+                    (commodity_id.clone(), region_id.clone(), ts.clone()),
+                    group_price,
+                )
+            })
+        })
+        .collect()
 }
 
 /// Calculate marginal cost prices for a set of commodities.
@@ -415,10 +484,9 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
-    // commodities, marginal costs are weighted by activity (or the activity limit for assets with
-    // no activity)
-    let mut existing_accum: IndexMap<_, WeightedAverageBackupAccumulator<MoneyPerFlow>> =
+    // For each (commodity, region, group), accumulate per-asset marginal costs.
+    // For seasonal/annual commodities, these marginal costs are weighted by activity.
+    let mut existing_accum: IndexMap<_, IndexMap<_, WeightedAverageBackupAccumulator>> =
         IndexMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
@@ -440,45 +508,33 @@ where
                 .time_slice_level
                 .containing_selection(time_slice);
 
-            // Insert the marginal cost into the accumulator for this group
-            let key = (
-                asset.clone(),
-                commodity_id.clone(),
-                region_id.clone(),
-                group,
-            );
-            let accum = existing_accum.entry(key).or_default();
-            accum.add(
-                marginal_cost,
-                Dimensionless(activity.value()),
-                Dimensionless(activity_limit.value()),
-            );
+            existing_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .entry(asset.clone())
+                .or_default()
+                .add(
+                    marginal_cost,
+                    Dimensionless(activity.value()),
+                    Dimensionless(activity_limit.value()),
+                );
         }
     }
 
-    // Compute per-group weighted-average marginal cost per asset, then take the max across assets
-    let mut group_prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
-    let mut priced_groups: HashSet<_> = HashSet::new();
-    for ((_, commodity_id, region_id, group), accum) in &existing_accum {
-        // Solve the weighted average marginal cost for this group
-        let Some(avg_cost) = accum.finalise() else {
-            continue;
-        };
-
-        // Take the max across assets for each group
-        group_prices
-            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
-            .and_modify(|c| *c = c.max(avg_cost))
-            .or_insert(avg_cost);
-        priced_groups.insert((commodity_id.clone(), region_id.clone(), group.clone()));
-    }
-
-    // Expand each group to individual time slices
-    let mut prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
-    expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
+    // For each group, finalise per-asset weighted averages then reduce to the max across assets
+    let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
+        .into_iter()
+        .filter_map(|(key, per_asset)| {
+            per_asset
+                .into_values()
+                .filter_map(|inner| inner.finalise())
+                .reduce(|current, value| current.max(value))
+                .map(|v| (key, v))
+        })
+        .collect();
 
     // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, WeightedAverageAccumulator<MoneyPerFlow>> = IndexMap::new();
+    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
@@ -499,38 +555,37 @@ where
                 .containing_selection(time_slice);
 
             // Skip groups already covered by existing assets
-            if priced_groups.contains(&(commodity_id.clone(), region_id.clone(), group.clone())) {
+            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
+            {
                 continue;
             }
 
-            let key = (
-                asset.clone(),
-                commodity_id.clone(),
-                region_id.clone(),
-                group,
-            );
-            let accum = cand_accum.entry(key).or_default();
-            accum.add(marginal_cost, Dimensionless(activity_limit.value()));
+            cand_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .entry(asset.clone())
+                .or_default()
+                .add(marginal_cost, Dimensionless(activity_limit.value()));
         }
     }
 
-    // Compute per-group weighted average per candidate, then take the min across candidates
-    // (i.e. the single most competitive candidate if a small amount of demand was added)
-    let mut cand_group_prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
-    for ((_, commodity_id, region_id, group), accum) in &cand_accum {
-        let Some(avg_cost) = accum.finalise() else {
-            continue;
-        };
-        cand_group_prices
-            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
-            .and_modify(|c| *c = c.min(avg_cost))
-            .or_insert(avg_cost);
-    }
+    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
+    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
+        .into_iter()
+        .filter_map(|(key, per_candidate)| {
+            per_candidate
+                .into_values()
+                .filter_map(|inner| inner.finalise())
+                .reduce(|current, value| current.min(value))
+                .map(|v| (key, v))
+        })
+        .collect();
 
-    // Expand candidate groups to individual time slices
-    expand_groups_to_prices(&cand_group_prices, time_slice_info, &mut prices);
+    // Merge existing and candidate group prices, then expand to individual time slices
+    let mut all_group_prices = group_prices;
+    all_group_prices.extend(cand_group_prices);
 
-    prices
+    expand_groups_to_prices(&all_group_prices, time_slice_info)
 }
 
 /// Calculate annual activities for each asset by summing across all time slices
@@ -624,10 +679,9 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // For each (asset, commodity, region, group), accumulate marginal costs. For seasonal/annual
-    // commodities, marginal costs are weighted by activity (or the activity limit for assets with
-    // no activity)
-    let mut existing_accum: IndexMap<_, WeightedAverageBackupAccumulator<MoneyPerFlow>> =
+    // For each (commodity, region, group), accumulate per-asset marginal costs.
+    // For seasonal/annual commodities, these marginal costs are weighted by activity.
+    let mut existing_accum: IndexMap<_, IndexMap<_, WeightedAverageBackupAccumulator>> =
         IndexMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
@@ -651,54 +705,52 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
+            // Get the group according to the commodity's time slice level
             let group = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
-            let key = (
-                asset.clone(),
-                commodity_id.clone(),
-                region_id.clone(),
-                group,
-            );
-            let accum = existing_accum.entry(key).or_default();
-            accum.add(
-                marginal_cost,
-                Dimensionless(activity.value()),
-                Dimensionless(activity_limit.value()),
-            );
+
+            existing_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .entry(asset.clone())
+                .or_default()
+                .add(
+                    marginal_cost,
+                    Dimensionless(activity.value()),
+                    Dimensionless(activity_limit.value()),
+                );
         }
     }
 
-    // Compute per-group weighted-average marginal cost per asset, add fixed costs, then take max
-    let mut group_prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
-    let mut priced_groups: HashSet<_> = HashSet::new();
-    let mut existing_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
-    for ((asset, commodity_id, region_id, group), accum) in &existing_accum {
-        // Solve the weighted average marginal cost for this group
-        let Some(avg_mc) = accum.finalise() else {
-            continue;
-        };
-
-        // Add fixed costs to get the full cost.
-        let annual_fixed_costs_per_flow = *existing_fixed_costs_cache
-            .entry(asset.clone())
-            .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activities[asset]));
-        let full_cost = avg_mc + annual_fixed_costs_per_flow;
-
-        // Take the max across assets for each group
-        group_prices
-            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
-            .and_modify(|c| *c = c.max(full_cost))
-            .or_insert(full_cost);
-        priced_groups.insert((commodity_id.clone(), region_id.clone(), group.clone()));
+    // Compute per-asset annual fixed costs for existing assets.
+    let mut existing_fixed_costs: HashMap<AssetRef, MoneyPerFlow> = HashMap::new();
+    for per_asset in existing_accum.values() {
+        for asset in per_asset.keys() {
+            existing_fixed_costs
+                .entry(asset.clone())
+                .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activities[asset]));
+        }
     }
 
-    // Expand each group to individual time slices
-    let mut prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
-    expand_groups_to_prices(&group_prices, time_slice_info, &mut prices);
+    // For each group, finalise per-asset weighted averages, add fixed costs, then reduce to max.
+    let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
+        .into_iter()
+        .filter_map(|(key, per_asset)| {
+            per_asset
+                .into_iter()
+                .filter_map(|(asset, inner)| {
+                    let avg_mc = inner.finalise()?;
+                    let annual_fixed_costs_per_flow = existing_fixed_costs[&asset];
+                    Some(avg_mc + annual_fixed_costs_per_flow)
+                })
+                .reduce(|current, value| current.max(value))
+                .map(|v| (key, v))
+        })
+        .collect();
 
     // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, WeightedAverageAccumulator<MoneyPerFlow>> = IndexMap::new();
+    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
@@ -719,54 +771,55 @@ where
                 .containing_selection(time_slice);
 
             // Skip groups already covered by existing assets
-            if priced_groups.contains(&(commodity_id.clone(), region_id.clone(), group.clone())) {
+            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
+            {
                 continue;
             }
 
-            let key = (
-                asset.clone(),
-                commodity_id.clone(),
-                region_id.clone(),
-                group,
-            );
-            let accum = cand_accum.entry(key).or_default();
-            accum.add(marginal_cost, Dimensionless(activity_limit.value()));
+            cand_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .entry(asset.clone())
+                .or_default()
+                .add(marginal_cost, Dimensionless(activity_limit.value()));
         }
     }
 
-    // Compute per-group weighted average, add fixed costs, take the min across candidates
-    let mut cand_fixed_costs_cache: HashMap<_, MoneyPerFlow> = HashMap::new();
-    let mut cand_group_prices: IndexMap<_, MoneyPerFlow> = IndexMap::new();
-    for ((asset, commodity_id, region_id, group), accum) in &cand_accum {
-        // Solve the weighted average marginal cost for this group
-        let Some(avg_mc) = accum.finalise() else {
-            continue;
-        };
-
-        // Add fixed costs to get the full cost. For candidates we assume full utilisation
-        let annual_fixed_costs_per_flow = *cand_fixed_costs_cache
-            .entry(asset.clone())
-            .or_insert_with(|| {
+    // Compute per-asset annual fixed costs for candidate assets at full utilisation.
+    let mut cand_fixed_costs: HashMap<AssetRef, MoneyPerFlow> = HashMap::new();
+    for per_candidate in cand_accum.values() {
+        for asset in per_candidate.keys() {
+            cand_fixed_costs.entry(asset.clone()).or_insert_with(|| {
                 asset.get_annual_fixed_costs_per_flow(
                     *asset
                         .get_activity_limits_for_selection(&TimeSliceSelection::Annual)
                         .end(),
                 )
             });
-        let full_cost = avg_mc + annual_fixed_costs_per_flow;
-
-        // Take the min across candidates for each group (i.e. the single most competitive candidate
-        // if a small amount of demand was added)
-        cand_group_prices
-            .entry((commodity_id.clone(), region_id.clone(), group.clone()))
-            .and_modify(|c| *c = c.min(full_cost))
-            .or_insert(full_cost);
+        }
     }
 
-    // Expand candidate groups to individual time slices
-    expand_groups_to_prices(&cand_group_prices, time_slice_info, &mut prices);
+    // For each group, finalise per-candidate weighted averages, add fixed costs, then reduce to min.
+    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
+        .into_iter()
+        .filter_map(|(key, per_candidate)| {
+            per_candidate
+                .into_iter()
+                .filter_map(|(asset, inner)| {
+                    let avg_mc = inner.finalise()?;
+                    let annual_fixed_costs_per_flow = cand_fixed_costs[&asset];
+                    Some(avg_mc + annual_fixed_costs_per_flow)
+                })
+                .reduce(|current, value| current.min(value))
+                .map(|v| (key, v))
+        })
+        .collect();
+
+    // Merge existing and candidate group prices, then expand to individual time slices
+    let mut all_group_prices = group_prices;
+    all_group_prices.extend(cand_group_prices);
 
-    prices
+    expand_groups_to_prices(&all_group_prices, time_slice_info)
 }
 
 #[cfg(test)]

From 4dccb937f9f64d8123e54aceb06be852d337c4b9 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Mon, 16 Mar 2026 20:52:32 +0000
Subject: [PATCH 12/23] Add new strategies

---
 src/simulation/prices.rs | 301 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 264 insertions(+), 37 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 3f4eb7b92..ed00df325 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -550,6 +550,110 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
+            // Get the group according to the commodity's time slice level
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+
+            // Skip groups already covered by existing assets
+            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
+            {
+                continue;
+            }
+
+            cand_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .entry(asset.clone())
+                .or_default()
+                .add(marginal_cost, Dimensionless(activity_limit.value()));
+        }
+    }
+
+    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
+    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
+        .into_iter()
+        .filter_map(|(key, per_candidate)| {
+            per_candidate
+                .into_values()
+                .filter_map(|inner| inner.finalise())
+                .reduce(|current, value| current.min(value))
+                .map(|v| (key, v))
+        })
+        .collect();
+
+    // Merge existing and candidate group prices, then expand to individual time slices
+    let mut all_group_prices = group_prices;
+    all_group_prices.extend(cand_group_prices);
+
+    expand_groups_to_prices(&all_group_prices, time_slice_info)
+}
+
+/// Similar to the above, but average across assets rather than taking the max for existing assets
+/// Candidate assets are treated the same way (take the min across candidates)
+fn calculate_marginal_cost_average_prices<'a, I, J>(
+    activity_for_existing: I,
+    activity_keys_for_candidates: J,
+    upstream_prices: &CommodityPrices,
+    year: u32,
+    markets_to_price: &HashSet<(CommodityID, RegionID)>,
+    commodities: &CommodityMap,
+    time_slice_info: &TimeSliceInfo,
+) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
+where
+    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
+    J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
+{
+    // For each (commodity, region, group), accumulate per-asset marginal costs.
+    // For seasonal/annual commodities, these marginal costs are weighted by activity.
+    // Marginal costs for different assets are averaged according to activity.
+    let mut existing_accum: IndexMap<_, WeightedAverageAccumulator> = IndexMap::new();
+    for (asset, time_slice, activity) in activity_for_existing {
+        let region_id = asset.region_id();
+
+        // Iterate over the marginal costs for commodities we need prices for
+        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
+            upstream_prices,
+            year,
+            time_slice,
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
+        ) {
+            // Get the group according to the commodity's time slice level
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+
+            existing_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .add(marginal_cost, Dimensionless(activity.value()));
+        }
+    }
+
+    // For each group, finalise per-asset weighted averages
+    let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
+        .into_iter()
+        .filter_map(|(key, accum)| accum.finalise().map(|v| (key, v)))
+        .collect();
+
+    // Candidate assets (assume full utilisation)
+    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
+    for (asset, time_slice) in activity_keys_for_candidates {
+        let region_id = asset.region_id();
+
+        // Get activity limits: used to weight marginal costs for seasonal/annual commodities
+        let activity_limit = *asset
+            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
+            .end();
+
+        // Iterate over the marginal costs for commodities we need prices for
+        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
+            upstream_prices,
+            year,
+            time_slice,
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
+        ) {
+            // Get the group according to the commodity's time slice level
             let group = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
@@ -683,6 +787,7 @@ where
     // For seasonal/annual commodities, these marginal costs are weighted by activity.
     let mut existing_accum: IndexMap<_, IndexMap<_, WeightedAverageBackupAccumulator>> =
         IndexMap::new();
+    let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
@@ -710,40 +815,31 @@ where
                 .time_slice_level
                 .containing_selection(time_slice);
 
+            // Get/calculate fixed costs per flow for this asset
+            let annual_fixed_costs_per_flow = annual_fixed_costs
+                .entry(asset.clone())
+                .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activity));
+
             existing_accum
                 .entry((commodity_id.clone(), region_id.clone(), group))
                 .or_default()
                 .entry(asset.clone())
                 .or_default()
                 .add(
-                    marginal_cost,
+                    marginal_cost + *annual_fixed_costs_per_flow,
                     Dimensionless(activity.value()),
                     Dimensionless(activity_limit.value()),
                 );
         }
     }
 
-    // Compute per-asset annual fixed costs for existing assets.
-    let mut existing_fixed_costs: HashMap<AssetRef, MoneyPerFlow> = HashMap::new();
-    for per_asset in existing_accum.values() {
-        for asset in per_asset.keys() {
-            existing_fixed_costs
-                .entry(asset.clone())
-                .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activities[asset]));
-        }
-    }
-
-    // For each group, finalise per-asset weighted averages, add fixed costs, then reduce to max.
+    // For each group, finalise per-asset weighted averages then reduce to the max across assets
     let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
         .into_iter()
         .filter_map(|(key, per_asset)| {
             per_asset
-                .into_iter()
-                .filter_map(|(asset, inner)| {
-                    let avg_mc = inner.finalise()?;
-                    let annual_fixed_costs_per_flow = existing_fixed_costs[&asset];
-                    Some(avg_mc + annual_fixed_costs_per_flow)
-                })
+                .into_values()
+                .filter_map(|inner| inner.finalise())
                 .reduce(|current, value| current.max(value))
                 .map(|v| (key, v))
         })
@@ -766,6 +862,7 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
+            // Get the group according to the commodity's time slice level
             let group = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
@@ -776,40 +873,170 @@ where
                 continue;
             }
 
+            // Get/calculate fixed costs per flow for this asset (assume full utilisation)
+            let annual_fixed_costs_per_flow =
+                annual_fixed_costs.entry(asset.clone()).or_insert_with(|| {
+                    asset.get_annual_fixed_costs_per_flow(
+                        *asset
+                            .get_activity_limits_for_selection(&TimeSliceSelection::Annual)
+                            .end(),
+                    )
+                });
+
             cand_accum
                 .entry((commodity_id.clone(), region_id.clone(), group))
                 .or_default()
                 .entry(asset.clone())
                 .or_default()
-                .add(marginal_cost, Dimensionless(activity_limit.value()));
+                .add(
+                    marginal_cost + *annual_fixed_costs_per_flow,
+                    Dimensionless(activity_limit.value()),
+                );
         }
     }
 
-    // Compute per-asset annual fixed costs for candidate assets at full utilisation.
-    let mut cand_fixed_costs: HashMap<AssetRef, MoneyPerFlow> = HashMap::new();
-    for per_candidate in cand_accum.values() {
-        for asset in per_candidate.keys() {
-            cand_fixed_costs.entry(asset.clone()).or_insert_with(|| {
-                asset.get_annual_fixed_costs_per_flow(
-                    *asset
-                        .get_activity_limits_for_selection(&TimeSliceSelection::Annual)
-                        .end(),
-                )
-            });
+    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
+    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
+        .into_iter()
+        .filter_map(|(key, per_candidate)| {
+            per_candidate
+                .into_values()
+                .filter_map(|inner| inner.finalise())
+                .reduce(|current, value| current.min(value))
+                .map(|v| (key, v))
+        })
+        .collect();
+
+    // Merge existing and candidate group prices, then expand to individual time slices
+    let mut all_group_prices = group_prices;
+    all_group_prices.extend(cand_group_prices);
+
+    expand_groups_to_prices(&all_group_prices, time_slice_info)
+}
+
+/// Similar to the above, but average across assets rather than taking the max for existing assets
+/// Candidate assets are treated the same way (take the min across candidates)
+#[allow(clippy::too_many_arguments)]
+fn calculate_full_cost_average_prices<'a, I, J>(
+    activity_for_existing: I,
+    activity_keys_for_candidates: J,
+    annual_activities: &HashMap<AssetRef, Activity>,
+    upstream_prices: &CommodityPrices,
+    year: u32,
+    markets_to_price: &HashSet<(CommodityID, RegionID)>,
+    commodities: &CommodityMap,
+    time_slice_info: &TimeSliceInfo,
+) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
+where
+    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
+    J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
+{
+    // For each (commodity, region, group), accumulate full costs.
+    // Costs are weighted across assets according to activity, and across time slices according to
+    // activity for seasonal/annual commodities.
+    let mut existing_accum: IndexMap<_, WeightedAverageAccumulator> = IndexMap::new();
+    let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
+    for (asset, time_slice, activity) in activity_for_existing {
+        let annual_activity = annual_activities[asset];
+        let region_id = asset.region_id();
+
+        // If annual activity is zero, we can't calculate a capital cost per flow, so skip this
+        // asset.
+        if annual_activity < Activity::EPSILON {
+            continue;
+        }
+
+        // Iterate over the marginal costs for commodities we need prices for
+        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
+            upstream_prices,
+            year,
+            time_slice,
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
+        ) {
+            // Get the group according to the commodity's time slice level
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+
+            // Get/calculate fixed costs per flow for this asset
+            let annual_fixed_costs_per_flow = annual_fixed_costs
+                .entry(asset.clone())
+                .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activity));
+
+            // Add marginal cost and fixed cost per flow, weighted by activity
+            existing_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .add(
+                    marginal_cost + *annual_fixed_costs_per_flow,
+                    Dimensionless(activity.value()),
+                );
+        }
+    }
+
+    // For each group, finalise weighted averages
+    let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
+        .into_iter()
+        .filter_map(|(key, accum)| accum.finalise().map(|v| (key, v)))
+        .collect();
+
+    // Candidate assets (assume full utilisation)
+    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
+    for (asset, time_slice) in activity_keys_for_candidates {
+        let region_id = asset.region_id();
+
+        // Get activity limits: used to weight marginal costs for seasonal/annual commodities
+        let activity_limit = *asset
+            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
+            .end();
+
+        // Iterate over the marginal costs for commodities we need prices for
+        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
+            upstream_prices,
+            year,
+            time_slice,
+            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
+        ) {
+            // Get the group according to the commodity's time slice level
+            let group = commodities[&commodity_id]
+                .time_slice_level
+                .containing_selection(time_slice);
+
+            // Skip groups already covered by existing assets
+            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
+            {
+                continue;
+            }
+
+            // Get/calculate fixed costs per flow for this asset (assume full utilisation)
+            let annual_fixed_costs_per_flow =
+                annual_fixed_costs.entry(asset.clone()).or_insert_with(|| {
+                    asset.get_annual_fixed_costs_per_flow(
+                        *asset
+                            .get_activity_limits_for_selection(&TimeSliceSelection::Annual)
+                            .end(),
+                    )
+                });
+
+            cand_accum
+                .entry((commodity_id.clone(), region_id.clone(), group))
+                .or_default()
+                .entry(asset.clone())
+                .or_default()
+                .add(
+                    marginal_cost + *annual_fixed_costs_per_flow,
+                    Dimensionless(activity_limit.value()),
+                );
         }
     }
 
-    // For each group, finalise per-candidate weighted averages, add fixed costs, then reduce to min.
+    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
     let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
         .into_iter()
         .filter_map(|(key, per_candidate)| {
             per_candidate
-                .into_iter()
-                .filter_map(|(asset, inner)| {
-                    let avg_mc = inner.finalise()?;
-                    let annual_fixed_costs_per_flow = cand_fixed_costs[&asset];
-                    Some(avg_mc + annual_fixed_costs_per_flow)
-                })
+                .into_values()
+                .filter_map(|inner| inner.finalise())
                 .reduce(|current, value| current.min(value))
                 .map(|v| (key, v))
         })

From eb2a2865e1b96d628b406ae635cd5e5ee8da960b Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Mon, 16 Mar 2026 21:14:43 +0000
Subject: [PATCH 13/23] Delete new strategies (for now)

---
 src/simulation/prices.rs | 238 ---------------------------------------
 1 file changed, 238 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index ed00df325..75b681d23 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -589,109 +589,6 @@ where
     expand_groups_to_prices(&all_group_prices, time_slice_info)
 }
 
-/// Similar to the above, but average across assets rather than taking the max for existing assets
-/// Candidate assets are treated the same way (take the min across candidates)
-fn calculate_marginal_cost_average_prices<'a, I, J>(
-    activity_for_existing: I,
-    activity_keys_for_candidates: J,
-    upstream_prices: &CommodityPrices,
-    year: u32,
-    markets_to_price: &HashSet<(CommodityID, RegionID)>,
-    commodities: &CommodityMap,
-    time_slice_info: &TimeSliceInfo,
-) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
-where
-    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
-    J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
-{
-    // For each (commodity, region, group), accumulate per-asset marginal costs.
-    // For seasonal/annual commodities, these marginal costs are weighted by activity.
-    // Marginal costs for different assets are averaged according to activity.
-    let mut existing_accum: IndexMap<_, WeightedAverageAccumulator> = IndexMap::new();
-    for (asset, time_slice, activity) in activity_for_existing {
-        let region_id = asset.region_id();
-
-        // Iterate over the marginal costs for commodities we need prices for
-        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
-            year,
-            time_slice,
-            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
-        ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
-                .time_slice_level
-                .containing_selection(time_slice);
-
-            existing_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
-                .or_default()
-                .add(marginal_cost, Dimensionless(activity.value()));
-        }
-    }
-
-    // For each group, finalise per-asset weighted averages
-    let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
-        .into_iter()
-        .filter_map(|(key, accum)| accum.finalise().map(|v| (key, v)))
-        .collect();
-
-    // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
-    for (asset, time_slice) in activity_keys_for_candidates {
-        let region_id = asset.region_id();
-
-        // Get activity limits: used to weight marginal costs for seasonal/annual commodities
-        let activity_limit = *asset
-            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
-            .end();
-
-        // Iterate over the marginal costs for commodities we need prices for
-        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
-            year,
-            time_slice,
-            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
-        ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
-                .time_slice_level
-                .containing_selection(time_slice);
-
-            // Skip groups already covered by existing assets
-            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
-            {
-                continue;
-            }
-
-            cand_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
-                .or_default()
-                .entry(asset.clone())
-                .or_default()
-                .add(marginal_cost, Dimensionless(activity_limit.value()));
-        }
-    }
-
-    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
-    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
-        .into_iter()
-        .filter_map(|(key, per_candidate)| {
-            per_candidate
-                .into_values()
-                .filter_map(|inner| inner.finalise())
-                .reduce(|current, value| current.min(value))
-                .map(|v| (key, v))
-        })
-        .collect();
-
-    // Merge existing and candidate group prices, then expand to individual time slices
-    let mut all_group_prices = group_prices;
-    all_group_prices.extend(cand_group_prices);
-
-    expand_groups_to_prices(&all_group_prices, time_slice_info)
-}
-
 /// Calculate annual activities for each asset by summing across all time slices
 fn calculate_annual_activities<'a, I>(activities: I) -> HashMap<AssetRef, Activity>
 where
@@ -914,141 +811,6 @@ where
     expand_groups_to_prices(&all_group_prices, time_slice_info)
 }
 
-/// Similar to the above, but average across assets rather than taking the max for existing assets
-/// Candidate assets are treated the same way (take the min across candidates)
-#[allow(clippy::too_many_arguments)]
-fn calculate_full_cost_average_prices<'a, I, J>(
-    activity_for_existing: I,
-    activity_keys_for_candidates: J,
-    annual_activities: &HashMap<AssetRef, Activity>,
-    upstream_prices: &CommodityPrices,
-    year: u32,
-    markets_to_price: &HashSet<(CommodityID, RegionID)>,
-    commodities: &CommodityMap,
-    time_slice_info: &TimeSliceInfo,
-) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
-where
-    I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
-    J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
-{
-    // For each (commodity, region, group), accumulate full costs.
-    // Costs are weighted across assets according to activity, and across time slices according to
-    // activity for seasonal/annual commodities.
-    let mut existing_accum: IndexMap<_, WeightedAverageAccumulator> = IndexMap::new();
-    let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
-    for (asset, time_slice, activity) in activity_for_existing {
-        let annual_activity = annual_activities[asset];
-        let region_id = asset.region_id();
-
-        // If annual activity is zero, we can't calculate a capital cost per flow, so skip this
-        // asset.
-        if annual_activity < Activity::EPSILON {
-            continue;
-        }
-
-        // Iterate over the marginal costs for commodities we need prices for
-        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
-            year,
-            time_slice,
-            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
-        ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
-                .time_slice_level
-                .containing_selection(time_slice);
-
-            // Get/calculate fixed costs per flow for this asset
-            let annual_fixed_costs_per_flow = annual_fixed_costs
-                .entry(asset.clone())
-                .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activity));
-
-            // Add marginal cost and fixed cost per flow, weighted by activity
-            existing_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
-                .or_default()
-                .add(
-                    marginal_cost + *annual_fixed_costs_per_flow,
-                    Dimensionless(activity.value()),
-                );
-        }
-    }
-
-    // For each group, finalise weighted averages
-    let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
-        .into_iter()
-        .filter_map(|(key, accum)| accum.finalise().map(|v| (key, v)))
-        .collect();
-
-    // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
-    for (asset, time_slice) in activity_keys_for_candidates {
-        let region_id = asset.region_id();
-
-        // Get activity limits: used to weight marginal costs for seasonal/annual commodities
-        let activity_limit = *asset
-            .get_activity_limits_for_selection(&TimeSliceSelection::Single(time_slice.clone()))
-            .end();
-
-        // Iterate over the marginal costs for commodities we need prices for
-        for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
-            year,
-            time_slice,
-            |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
-        ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
-                .time_slice_level
-                .containing_selection(time_slice);
-
-            // Skip groups already covered by existing assets
-            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
-            {
-                continue;
-            }
-
-            // Get/calculate fixed costs per flow for this asset (assume full utilisation)
-            let annual_fixed_costs_per_flow =
-                annual_fixed_costs.entry(asset.clone()).or_insert_with(|| {
-                    asset.get_annual_fixed_costs_per_flow(
-                        *asset
-                            .get_activity_limits_for_selection(&TimeSliceSelection::Annual)
-                            .end(),
-                    )
-                });
-
-            cand_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
-                .or_default()
-                .entry(asset.clone())
-                .or_default()
-                .add(
-                    marginal_cost + *annual_fixed_costs_per_flow,
-                    Dimensionless(activity_limit.value()),
-                );
-        }
-    }
-
-    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
-    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
-        .into_iter()
-        .filter_map(|(key, per_candidate)| {
-            per_candidate
-                .into_values()
-                .filter_map(|inner| inner.finalise())
-                .reduce(|current, value| current.min(value))
-                .map(|v| (key, v))
-        })
-        .collect();
-
-    // Merge existing and candidate group prices, then expand to individual time slices
-    let mut all_group_prices = group_prices;
-    all_group_prices.extend(cand_group_prices);
-
-    expand_groups_to_prices(&all_group_prices, time_slice_info)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;

From dbd888bcb348b84a69141f068677a6c92127b8a0 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Tue, 17 Mar 2026 09:59:55 +0000
Subject: [PATCH 14/23] Tests and tidy ups

---
 src/simulation/prices.rs | 176 ++++++++++++++++++++++++++++++---------
 1 file changed, 138 insertions(+), 38 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 75b681d23..9c242733c 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -10,10 +10,12 @@ use anyhow::Result;
 use indexmap::IndexMap;
 use std::collections::{HashMap, HashSet};
 
-/// Weighted average accumulator for `MoneyPerFlow`.
+/// Weighted average accumulator for `MoneyPerFlow` prices.
 #[derive(Clone, Copy, Debug)]
 struct WeightedAverageAccumulator {
+    /// The numerator of the weighted average, i.e. the sum of value * weight across all entries.
     numerator: MoneyPerFlow,
+    /// The denominator of the weighted average, i.e. the sum of weights across all entries.
     denominator: Dimensionless,
 }
 
@@ -27,24 +29,30 @@ impl Default for WeightedAverageAccumulator {
 }
 
 impl WeightedAverageAccumulator {
-    /// Add a weighted value.
+    /// Add a weighted value to the accumulator.
     fn add(&mut self, value: MoneyPerFlow, weight: Dimensionless) {
         self.numerator += value * weight;
         self.denominator += weight;
     }
 
     /// Solve the weighted average.
+    ///
+    /// Returns `None` if the denominator is zero (or close to zero)
     fn finalise(&self) -> Option<MoneyPerFlow> {
         (self.denominator > Dimensionless::EPSILON).then(|| self.numerator / self.denominator)
     }
 }
 
-/// Weighted average accumulator with a backup weighting path for `MoneyPerFlow`.
+/// Weighted average accumulator with a backup weighting path for `MoneyPerFlow` prices.
 #[derive(Clone, Copy, Debug)]
 struct WeightedAverageBackupAccumulator {
+    /// The numerator of the primary weighted average, i.e. the sum of value * weight across all entries.
     primary_numerator: MoneyPerFlow,
+    /// The denominator of the primary weighted average, i.e. the sum of weights across all entries.
     primary_denominator: Dimensionless,
+    /// The numerator of the backup weighted average, i.e. the sum of value * weight across all entries.
     backup_numerator: MoneyPerFlow,
+    /// The denominator of the backup weighted average, i.e. the sum of weights across all entries.
     backup_denominator: Dimensionless,
 }
 
@@ -60,7 +68,7 @@ impl Default for WeightedAverageBackupAccumulator {
 }
 
 impl WeightedAverageBackupAccumulator {
-    /// Add a weighted value with a backup weight.
+    /// Add a weighted value to the accumulator with a backup weight.
     fn add(&mut self, value: MoneyPerFlow, weight: Dimensionless, backup_weight: Dimensionless) {
         self.primary_numerator += value * weight;
         self.primary_denominator += weight;
@@ -69,6 +77,8 @@ impl WeightedAverageBackupAccumulator {
     }
 
     /// Solve the weighted average, falling back to backup weights if needed.
+    ///
+    /// Returns `None` if both denominators are zero (or close to zero).
     fn finalise(&self) -> Option<MoneyPerFlow> {
         if self.primary_denominator > Dimensionless::EPSILON {
             Some(self.primary_numerator / self.primary_denominator)
@@ -400,8 +410,10 @@ where
     scarcity_prices
 }
 
-/// Expand a map of per-group prices to individual time slices.
-fn expand_groups_to_prices(
+/// Expand a map of prices for commodity/region/time slice selections to a map of prices for
+/// commodity/region/time slices by applying the same price to all time slices within each
+/// selection.
+fn expand_selection_prices(
     group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
     time_slice_info: &TimeSliceInfo,
 ) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow> {
@@ -484,10 +496,17 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // For each (commodity, region, group), accumulate per-asset marginal costs.
-    // For seasonal/annual commodities, these marginal costs are weighted by activity.
-    let mut existing_accum: IndexMap<_, IndexMap<_, WeightedAverageBackupAccumulator>> =
-        IndexMap::new();
+    // Accumulator map to collect marginal costs from existing assets. For each (commodity, region,
+    // ts selection), this maps each asset to a weighted average of the marginal costs for that
+    // commodity across all time slices in the selection, weighted by activity (using activity
+    // limits as a backup weight if there is zero activity across the selection). The granularity of
+    // the selection depends on the time slice level of the commodity (i.e. individual, season, year).
+    let mut existing_accum: IndexMap<
+        (CommodityID, RegionID, TimeSliceSelection),
+        IndexMap<AssetRef, WeightedAverageBackupAccumulator>,
+    > = IndexMap::new();
+
+    // Iterate over existing assets and their activities
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
 
@@ -503,13 +522,15 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
+            // Get the time slice selection according to the commodity's time slice level
+            let ts_selection = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
 
+            // Accumulate marginal cost for this asset, weighted by activity (using the activity
+            // limit as a backup weight)
             existing_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
+                .entry((commodity_id.clone(), region_id.clone(), ts_selection))
                 .or_default()
                 .entry(asset.clone())
                 .or_default()
@@ -521,7 +542,7 @@ where
         }
     }
 
-    // For each group, finalise per-asset weighted averages then reduce to the max across assets
+    // For each group, finalise per-asset weighted averages then take the max across assets
     let group_prices: IndexMap<_, MoneyPerFlow> = existing_accum
         .into_iter()
         .filter_map(|(key, per_asset)| {
@@ -533,8 +554,14 @@ where
         })
         .collect();
 
-    // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
+    // Accumulator map to collect marginal costs from candidate assets. Similar to existing_accum,
+    // but costs are weighted according to activity limits (i.e. assuming full utilisation).
+    let mut cand_accum: IndexMap<
+        (CommodityID, RegionID, TimeSliceSelection),
+        IndexMap<AssetRef, WeightedAverageAccumulator>,
+    > = IndexMap::new();
+
+    // Iterate over candidate assets (assuming full utilization)
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
@@ -550,19 +577,23 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
+            // Get the time slice selection according to the commodity's time slice level
+            let ts_selection = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
 
             // Skip groups already covered by existing assets
-            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
-            {
+            if group_prices.contains_key(&(
+                commodity_id.clone(),
+                region_id.clone(),
+                ts_selection.clone(),
+            )) {
                 continue;
             }
 
+            // Accumulate marginal cost for this candidate asset, weighted by the activity limit
             cand_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
+                .entry((commodity_id.clone(), region_id.clone(), ts_selection))
                 .or_default()
                 .entry(asset.clone())
                 .or_default()
@@ -570,7 +601,7 @@ where
         }
     }
 
-    // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
+    // For each group, finalise per-candidate weighted averages then take the min across candidates
     let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
         .into_iter()
         .filter_map(|(key, per_candidate)| {
@@ -586,7 +617,7 @@ where
     let mut all_group_prices = group_prices;
     all_group_prices.extend(cand_group_prices);
 
-    expand_groups_to_prices(&all_group_prices, time_slice_info)
+    expand_selection_prices(&all_group_prices, time_slice_info)
 }
 
 /// Calculate annual activities for each asset by summing across all time slices
@@ -680,11 +711,20 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
-    // For each (commodity, region, group), accumulate per-asset marginal costs.
-    // For seasonal/annual commodities, these marginal costs are weighted by activity.
-    let mut existing_accum: IndexMap<_, IndexMap<_, WeightedAverageBackupAccumulator>> =
-        IndexMap::new();
+    // Accumulator map to collect full costs from existing assets. For each (commodity, region,
+    // ts selection), this maps each asset to a weighted average of the full costs for that
+    // commodity across all time slices in the selection, weighted by activity (using activity
+    // limits as a backup weight if there is zero activity across the selection). The granularity of
+    // the selection depends on the time slice level of the commodity (i.e. individual, season, year).
+    let mut existing_accum: IndexMap<
+        (CommodityID, RegionID, TimeSliceSelection),
+        IndexMap<AssetRef, WeightedAverageBackupAccumulator>,
+    > = IndexMap::new();
+
+    // Cache of annual fixed costs per flow for each asset, to avoid recalculating
     let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
+
+    // Iterate over existing assets and their activities
     for (asset, time_slice, activity) in activity_for_existing {
         let annual_activity = annual_activities[asset];
         let region_id = asset.region_id();
@@ -707,8 +747,8 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
+            // Get the time slice selection according to the commodity's time slice level
+            let ts_selection = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
 
@@ -717,8 +757,10 @@ where
                 .entry(asset.clone())
                 .or_insert_with(|| asset.get_annual_fixed_costs_per_flow(annual_activity));
 
+            // Accumulate full cost for this asset, weighted by activity (using the activity limit
+            // as a backup weight)
             existing_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
+                .entry((commodity_id.clone(), region_id.clone(), ts_selection))
                 .or_default()
                 .entry(asset.clone())
                 .or_default()
@@ -742,8 +784,14 @@ where
         })
         .collect();
 
-    // Candidate assets (assume full utilisation)
-    let mut cand_accum: IndexMap<_, IndexMap<_, WeightedAverageAccumulator>> = IndexMap::new();
+    // Accumulator map to collect full costs from candidate assets. Similar to existing_accum, but
+    // costs are weighted according to activity limits (i.e. assuming full utilisation).
+    let mut cand_accum: IndexMap<
+        (CommodityID, RegionID, TimeSliceSelection),
+        IndexMap<AssetRef, WeightedAverageAccumulator>,
+    > = IndexMap::new();
+
+    // Iterate over candidate assets (assuming full utilization)
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 
@@ -759,14 +807,17 @@ where
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
         ) {
-            // Get the group according to the commodity's time slice level
-            let group = commodities[&commodity_id]
+            // Get the time slice selection according to the commodity's time slice level
+            let ts_selection = commodities[&commodity_id]
                 .time_slice_level
                 .containing_selection(time_slice);
 
             // Skip groups already covered by existing assets
-            if group_prices.contains_key(&(commodity_id.clone(), region_id.clone(), group.clone()))
-            {
+            if group_prices.contains_key(&(
+                commodity_id.clone(),
+                region_id.clone(),
+                ts_selection.clone(),
+            )) {
                 continue;
             }
 
@@ -780,8 +831,9 @@ where
                     )
                 });
 
+            // Accumulate full cost for this candidate asset, weighted by the activity limit
             cand_accum
-                .entry((commodity_id.clone(), region_id.clone(), group))
+                .entry((commodity_id.clone(), region_id.clone(), ts_selection))
                 .or_default()
                 .entry(asset.clone())
                 .or_default()
@@ -808,7 +860,7 @@ where
     let mut all_group_prices = group_prices;
     all_group_prices.extend(cand_group_prices);
 
-    expand_groups_to_prices(&all_group_prices, time_slice_info)
+    expand_selection_prices(&all_group_prices, time_slice_info)
 }
 
 #[cfg(test)]
@@ -1108,4 +1160,52 @@ mod tests {
         assert_price_approx(&prices, &b.id, &region_id, &time_slice, MoneyPerFlow(5.0));
         assert_price_approx(&prices, &c.id, &region_id, &time_slice, MoneyPerFlow(8.0));
     }
+
+    #[test]
+    fn weighted_average_accumulator_single_value() {
+        let mut accum = WeightedAverageAccumulator::default();
+        accum.add(MoneyPerFlow(100.0), Dimensionless(1.0));
+        assert_eq!(accum.finalise(), Some(MoneyPerFlow(100.0)));
+    }
+
+    #[test]
+    fn weighted_average_accumulator_different_weights() {
+        let mut accum = WeightedAverageAccumulator::default();
+        accum.add(MoneyPerFlow(100.0), Dimensionless(1.0));
+        accum.add(MoneyPerFlow(200.0), Dimensionless(2.0));
+        // (100*1 + 200*2) / (1+2) = 500/3 ≈ 166.667
+        let result = accum.finalise().unwrap();
+        assert!((result - MoneyPerFlow(500.0 / 3.0)).abs() < MoneyPerFlow::EPSILON);
+    }
+
+    #[test]
+    fn weighted_average_accumulator_zero_weight() {
+        let accum = WeightedAverageAccumulator::default();
+        assert_eq!(accum.finalise(), None);
+    }
+
+    #[test]
+    fn weighted_average_backup_accumulator_primary_preferred() {
+        let mut accum = WeightedAverageBackupAccumulator::default();
+        accum.add(MoneyPerFlow(100.0), Dimensionless(3.0), Dimensionless(1.0));
+        accum.add(MoneyPerFlow(200.0), Dimensionless(1.0), Dimensionless(1.0));
+        // Primary is non-zero, use it: (100*3 + 200*1) / (3+1) = 125
+        // (backup would be (100*1 + 200*1) / (1+1) = 150, but we don't use it)
+        assert_eq!(accum.finalise(), Some(MoneyPerFlow(125.0)));
+    }
+
+    #[test]
+    fn weighted_average_backup_accumulator_fallback() {
+        let mut accum = WeightedAverageBackupAccumulator::default();
+        accum.add(MoneyPerFlow(100.0), Dimensionless(0.0), Dimensionless(2.0));
+        accum.add(MoneyPerFlow(200.0), Dimensionless(0.0), Dimensionless(2.0));
+        // Primary is zero, fallback to backup: (100*2 + 200*2) / (2+2) = 150
+        assert_eq!(accum.finalise(), Some(MoneyPerFlow(150.0)));
+    }
+
+    #[test]
+    fn weighted_average_backup_accumulator_both_zero() {
+        let accum = WeightedAverageBackupAccumulator::default();
+        assert_eq!(accum.finalise(), None);
+    }
 }

From 7eb6a28d04bb476e436d03821505679a4253eb2f Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Tue, 17 Mar 2026 10:23:00 +0000
Subject: [PATCH 15/23] Make expand_selection_prices safer

---
 src/simulation/prices.rs | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 9c242733c..be6c2e340 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -209,7 +209,8 @@ impl CommodityPrices {
         price: MoneyPerFlow,
     ) {
         let key = (commodity_id.clone(), region_id.clone(), time_slice.clone());
-        self.0.insert(key, price);
+        let existing = self.0.insert(key.clone(), price).is_some();
+        assert!(!existing, "Key {key:?} already exists in the map");
     }
 
     /// Extend the prices map, panic if any key already exists
@@ -417,17 +418,21 @@ fn expand_selection_prices(
     group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
     time_slice_info: &TimeSliceInfo,
 ) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow> {
-    group_prices
-        .iter()
-        .flat_map(|((commodity_id, region_id, group), &group_price)| {
-            group.iter(time_slice_info).map(move |(ts, _)| {
-                (
-                    (commodity_id.clone(), region_id.clone(), ts.clone()),
-                    group_price,
-                )
-            })
-        })
-        .collect()
+    let mut prices = IndexMap::new();
+
+    for ((commodity_id, region_id, selection), &selection_price) in group_prices {
+        for (time_slice_id, _) in selection.iter(time_slice_info) {
+            let key = (
+                commodity_id.clone(),
+                region_id.clone(),
+                time_slice_id.clone(),
+            );
+            let existing = prices.insert(key.clone(), selection_price).is_some();
+            assert!(!existing, "Key {key:?} already exists in the map");
+        }
+    }
+
+    prices
 }
 
 /// Calculate marginal cost prices for a set of commodities.

From 5b26fac2e10a4cecc52b6ddf1cfcb3e1d041ddfd Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Tue, 17 Mar 2026 14:34:20 +0000
Subject: [PATCH 16/23] Use WeightedAverageAccumulator in
 WeightedAverageBackupAccumulator

---
 src/simulation/prices.rs | 39 ++++++++-------------------------------
 1 file changed, 8 insertions(+), 31 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index be6c2e340..d4f416308 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -44,49 +44,26 @@ impl WeightedAverageAccumulator {
 }
 
 /// Weighted average accumulator with a backup weighting path for `MoneyPerFlow` prices.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, Default)]
 struct WeightedAverageBackupAccumulator {
-    /// The numerator of the primary weighted average, i.e. the sum of value * weight across all entries.
-    primary_numerator: MoneyPerFlow,
-    /// The denominator of the primary weighted average, i.e. the sum of weights across all entries.
-    primary_denominator: Dimensionless,
-    /// The numerator of the backup weighted average, i.e. the sum of value * weight across all entries.
-    backup_numerator: MoneyPerFlow,
-    /// The denominator of the backup weighted average, i.e. the sum of weights across all entries.
-    backup_denominator: Dimensionless,
-}
-
-impl Default for WeightedAverageBackupAccumulator {
-    fn default() -> Self {
-        Self {
-            primary_numerator: MoneyPerFlow(0.0),
-            primary_denominator: Dimensionless(0.0),
-            backup_numerator: MoneyPerFlow(0.0),
-            backup_denominator: Dimensionless(0.0),
-        }
-    }
+    /// Primary weighted average path.
+    primary: WeightedAverageAccumulator,
+    /// Backup weighted average path.
+    backup: WeightedAverageAccumulator,
 }
 
 impl WeightedAverageBackupAccumulator {
     /// Add a weighted value to the accumulator with a backup weight.
     fn add(&mut self, value: MoneyPerFlow, weight: Dimensionless, backup_weight: Dimensionless) {
-        self.primary_numerator += value * weight;
-        self.primary_denominator += weight;
-        self.backup_numerator += value * backup_weight;
-        self.backup_denominator += backup_weight;
+        self.primary.add(value, weight);
+        self.backup.add(value, backup_weight);
     }
 
     /// Solve the weighted average, falling back to backup weights if needed.
     ///
     /// Returns `None` if both denominators are zero (or close to zero).
     fn finalise(&self) -> Option<MoneyPerFlow> {
-        if self.primary_denominator > Dimensionless::EPSILON {
-            Some(self.primary_numerator / self.primary_denominator)
-        } else if self.backup_denominator > Dimensionless::EPSILON {
-            Some(self.backup_numerator / self.backup_denominator)
-        } else {
-            None
-        }
+        self.primary.finalise().or_else(|| self.backup.finalise())
     }
 }
 

From 2e177d5ac4de2e3d40a2351a4f821e1d1a9243d4 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Wed, 18 Mar 2026 10:21:08 +0000
Subject: [PATCH 17/23] Implement easy suggestions

---
 src/simulation/prices.rs | 67 +++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index d4f416308..6dba0513b 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -38,7 +38,7 @@ impl WeightedAverageAccumulator {
     /// Solve the weighted average.
     ///
     /// Returns `None` if the denominator is zero (or close to zero)
-    fn finalise(&self) -> Option<MoneyPerFlow> {
+    fn finalise(self) -> Option<MoneyPerFlow> {
         (self.denominator > Dimensionless::EPSILON).then(|| self.numerator / self.denominator)
     }
 }
@@ -62,7 +62,7 @@ impl WeightedAverageBackupAccumulator {
     /// Solve the weighted average, falling back to backup weights if needed.
     ///
     /// Returns `None` if both denominators are zero (or close to zero).
-    fn finalise(&self) -> Option<MoneyPerFlow> {
+    fn finalise(self) -> Option<MoneyPerFlow> {
         self.primary.finalise().or_else(|| self.backup.finalise())
     }
 }
@@ -388,15 +388,13 @@ where
     scarcity_prices
 }
 
-/// Expand a map of prices for commodity/region/time slice selections to a map of prices for
-/// commodity/region/time slices by applying the same price to all time slices within each
-/// selection.
-fn expand_selection_prices(
+/// Extend an existing commodity/region/time-slice price map by applying each
+/// selection-level price to all time slices within that selection.
+fn extend_selection_prices(
+    prices: &mut IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>,
     group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
     time_slice_info: &TimeSliceInfo,
-) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow> {
-    let mut prices = IndexMap::new();
-
+) {
     for ((commodity_id, region_id, selection), &selection_price) in group_prices {
         for (time_slice_id, _) in selection.iter(time_slice_info) {
             let key = (
@@ -408,8 +406,6 @@ fn expand_selection_prices(
             assert!(!existing, "Key {key:?} already exists in the map");
         }
     }
-
-    prices
 }
 
 /// Calculate marginal cost prices for a set of commodities.
@@ -530,7 +526,7 @@ where
         .filter_map(|(key, per_asset)| {
             per_asset
                 .into_values()
-                .filter_map(|inner| inner.finalise())
+                .filter_map(WeightedAverageBackupAccumulator::finalise)
                 .reduce(|current, value| current.max(value))
                 .map(|v| (key, v))
         })
@@ -584,22 +580,21 @@ where
     }
 
     // For each group, finalise per-candidate weighted averages then take the min across candidates
-    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
-        .into_iter()
-        .filter_map(|(key, per_candidate)| {
-            per_candidate
-                .into_values()
-                .filter_map(|inner| inner.finalise())
-                .reduce(|current, value| current.min(value))
-                .map(|v| (key, v))
-        })
-        .collect();
+    let cand_group_prices = cand_accum.into_iter().filter_map(|(key, per_candidate)| {
+        per_candidate
+            .into_values()
+            .filter_map(WeightedAverageAccumulator::finalise)
+            .reduce(|current, value| current.min(value))
+            .map(|v| (key, v))
+    });
 
     // Merge existing and candidate group prices, then expand to individual time slices
     let mut all_group_prices = group_prices;
     all_group_prices.extend(cand_group_prices);
 
-    expand_selection_prices(&all_group_prices, time_slice_info)
+    let mut prices = IndexMap::new();
+    extend_selection_prices(&mut prices, &all_group_prices, time_slice_info);
+    prices
 }
 
 /// Calculate annual activities for each asset by summing across all time slices
@@ -760,7 +755,7 @@ where
         .filter_map(|(key, per_asset)| {
             per_asset
                 .into_values()
-                .filter_map(|inner| inner.finalise())
+                .filter_map(WeightedAverageBackupAccumulator::finalise)
                 .reduce(|current, value| current.max(value))
                 .map(|v| (key, v))
         })
@@ -827,22 +822,21 @@ where
     }
 
     // For each group, finalise per-candidate weighted averages then reduce to the min across candidates
-    let cand_group_prices: IndexMap<_, MoneyPerFlow> = cand_accum
-        .into_iter()
-        .filter_map(|(key, per_candidate)| {
-            per_candidate
-                .into_values()
-                .filter_map(|inner| inner.finalise())
-                .reduce(|current, value| current.min(value))
-                .map(|v| (key, v))
-        })
-        .collect();
+    let cand_group_prices = cand_accum.into_iter().filter_map(|(key, per_candidate)| {
+        per_candidate
+            .into_values()
+            .filter_map(WeightedAverageAccumulator::finalise)
+            .reduce(|current, value| current.min(value))
+            .map(|v| (key, v))
+    });
 
     // Merge existing and candidate group prices, then expand to individual time slices
     let mut all_group_prices = group_prices;
     all_group_prices.extend(cand_group_prices);
 
-    expand_selection_prices(&all_group_prices, time_slice_info)
+    let mut prices = IndexMap::new();
+    extend_selection_prices(&mut prices, &all_group_prices, time_slice_info);
+    prices
 }
 
 #[cfg(test)]
@@ -862,6 +856,7 @@ mod tests {
         Activity, Capacity, Dimensionless, FlowPerActivity, MoneyPerActivity, MoneyPerCapacity,
         MoneyPerCapacityPerYear, MoneyPerFlow,
     };
+    use float_cmp::assert_approx_eq;
     use indexmap::{IndexMap, IndexSet};
     use rstest::rstest;
     use std::collections::{HashMap, HashSet};
@@ -1157,7 +1152,7 @@ mod tests {
         accum.add(MoneyPerFlow(200.0), Dimensionless(2.0));
         // (100*1 + 200*2) / (1+2) = 500/3 ≈ 166.667
         let result = accum.finalise().unwrap();
-        assert!((result - MoneyPerFlow(500.0 / 3.0)).abs() < MoneyPerFlow::EPSILON);
+        assert_approx_eq!(MoneyPerFlow, result, MoneyPerFlow(500.0 / 3.0));
     }
 
     #[test]

From 7c2e4602d3ae76a065aef3e5260e74d95a66157c Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Wed, 18 Mar 2026 10:54:41 +0000
Subject: [PATCH 18/23] Add prices to existing maps rather than creating new
 maps

---
 src/simulation/prices.rs | 117 +++++++++++++++------------------------
 1 file changed, 46 insertions(+), 71 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 6dba0513b..a4ab5d0b0 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -127,26 +127,25 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
 
         // Add prices for scarcity-adjusted commodities
         if let Some(scarcity_set) = pricing_sets.get(&PricingStrategy::ScarcityAdjusted) {
-            let scarcity_prices = calculate_scarcity_adjusted_prices(
+            add_scarcity_adjusted_prices(
                 solution.iter_activity_duals(),
                 &shadow_prices,
+                &mut result,
                 scarcity_set,
             );
-            result.extend(scarcity_prices);
         }
 
         // Add prices for marginal cost commodities
         if let Some(marginal_set) = pricing_sets.get(&PricingStrategy::MarginalCost) {
-            let marginal_cost_prices = calculate_marginal_cost_prices(
+            add_marginal_cost_prices(
                 solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
-                &result,
+                &mut result,
                 year,
                 marginal_set,
                 &model.commodities,
                 &model.time_slice_info,
             );
-            result.extend(marginal_cost_prices);
         }
 
         // Add prices for full cost commodities
@@ -154,17 +153,16 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
             let annual_activities = annual_activities.get_or_insert_with(|| {
                 calculate_annual_activities(solution.iter_activity_for_existing())
             });
-            let full_cost_prices = calculate_full_cost_prices(
+            add_full_cost_prices(
                 solution.iter_activity_for_existing(),
                 solution.iter_activity_keys_for_candidates(),
                 annual_activities,
-                &result,
+                &mut result,
                 year,
                 fullcost_set,
                 &model.commodities,
                 &model.time_slice_info,
             );
-            result.extend(full_cost_prices);
         }
     }
 
@@ -325,23 +323,20 @@ impl IntoIterator for CommodityPrices {
     }
 }
 
-/// Calculate scarcity-adjusted prices for a set of commodities.
+/// Calculate scarcity-adjusted prices for a set of commodities and add to an existing prices map.
 ///
 /// # Arguments
 ///
 /// * `activity_duals` - Iterator over activity duals from optimisation solution
 /// * `shadow_prices` - Shadow prices for all commodities
+/// * `existing_prices` - Existing prices map to extend with scarcity-adjusted prices
 /// * `markets_to_price` - Set of markets to calculate scarcity-adjusted prices for
-///
-/// # Returns
-///
-/// A map of scarcity-adjusted prices for the specified markets in all time slices
-fn calculate_scarcity_adjusted_prices<'a, I>(
+fn add_scarcity_adjusted_prices<'a, I>(
     activity_duals: I,
     shadow_prices: &CommodityPrices,
+    existing_prices: &mut CommodityPrices,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
-) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
-where
+) where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, MoneyPerActivity)>,
 {
     // Calculate highest activity dual for each commodity/region/time slice
@@ -370,8 +365,7 @@ where
         }
     }
 
-    // Add this to the shadow price for each commodity/region/time slice
-    let mut scarcity_prices = IndexMap::new();
+    // Add this to the shadow price for each commodity/region/time slice and insert into the map
     for ((commodity, region, time_slice), highest_dual) in &highest_duals {
         // There should always be a shadow price for commodities we are considering here, so it
         // should be safe to unwrap
@@ -379,36 +373,25 @@ where
         // highest_dual is in units of MoneyPerActivity, and shadow_price is in MoneyPerFlow, but
         // this is correct according to Adam
         let scarcity_price = shadow_price + MoneyPerFlow(highest_dual.value());
-        scarcity_prices.insert(
-            (commodity.clone(), region.clone(), time_slice.clone()),
-            scarcity_price,
-        );
+        existing_prices.insert(commodity, region, time_slice, scarcity_price);
     }
-
-    scarcity_prices
 }
 
 /// Extend an existing commodity/region/time-slice price map by applying each
 /// selection-level price to all time slices within that selection.
 fn extend_selection_prices(
-    prices: &mut IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>,
+    prices: &mut CommodityPrices,
     group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
     time_slice_info: &TimeSliceInfo,
 ) {
     for ((commodity_id, region_id, selection), &selection_price) in group_prices {
         for (time_slice_id, _) in selection.iter(time_slice_info) {
-            let key = (
-                commodity_id.clone(),
-                region_id.clone(),
-                time_slice_id.clone(),
-            );
-            let existing = prices.insert(key.clone(), selection_price).is_some();
-            assert!(!existing, "Key {key:?} already exists in the map");
+            prices.insert(commodity_id, region_id, time_slice_id, selection_price);
         }
     }
 }
 
-/// Calculate marginal cost prices for a set of commodities.
+/// Calculate marginal cost prices for a set of commodities and add to an existing prices map.
 ///
 /// This pricing strategy aims to incorporate the marginal cost of commodity production into the price.
 ///
@@ -452,25 +435,21 @@ fn extend_selection_prices(
 /// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
 ///   solution for existing assets
 /// * `activity_keys_for_candidates` - Iterator over `(asset, time_slice)` for candidate assets
-/// * `upstream_prices` - Prices for commodities upstream of the ones we are calculating prices for
+/// * `existing_prices` - Existing prices to use as inputs and extend. This is expected to include
+///   prices from all markets upstream of the markets we are calculating for.
 /// * `year` - The year for which prices are being calculated
 /// * `markets_to_price` - Set of markets to calculate marginal prices for
 /// * `commodities` - Map of all commodities (used to look up each commodity's `time_slice_level`)
 /// * `time_slice_info` - Time slice information (used to expand groups to individual time slices)
-///
-/// # Returns
-///
-/// A map of marginal cost prices for the specified markets in all time slices
-fn calculate_marginal_cost_prices<'a, I, J>(
+fn add_marginal_cost_prices<'a, I, J>(
     activity_for_existing: I,
     activity_keys_for_candidates: J,
-    upstream_prices: &CommodityPrices,
+    existing_prices: &mut CommodityPrices,
     year: u32,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
     commodities: &CommodityMap,
     time_slice_info: &TimeSliceInfo,
-) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
-where
+) where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
@@ -495,7 +474,7 @@ where
 
         // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
+            existing_prices,
             year,
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
@@ -550,7 +529,7 @@ where
 
         // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
+            existing_prices,
             year,
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
@@ -588,13 +567,12 @@ where
             .map(|v| (key, v))
     });
 
-    // Merge existing and candidate group prices, then expand to individual time slices
+    // Merge existing and candidate group prices
     let mut all_group_prices = group_prices;
     all_group_prices.extend(cand_group_prices);
 
-    let mut prices = IndexMap::new();
-    extend_selection_prices(&mut prices, &all_group_prices, time_slice_info);
-    prices
+    // Expand selection-level prices to individual time slices and add to the main prices map
+    extend_selection_prices(existing_prices, &all_group_prices, time_slice_info);
 }
 
 /// Calculate annual activities for each asset by summing across all time slices
@@ -613,7 +591,7 @@ where
         })
 }
 
-/// Calculate full cost prices for a set of commodities.
+/// Calculate full cost prices for a set of commodities and add to an existing prices map.
 ///
 /// This pricing strategy aims to incorporate the full cost of commodity production into the price.
 ///
@@ -664,27 +642,23 @@ where
 /// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
 ///   solution for existing assets
 /// * `activity_keys_for_candidates` - Iterator over `(asset, time_slice)` for candidate assets
-/// * `upstream_prices` - Prices for commodities upstream of the ones we are calculating prices for
+/// * `existing_prices` - Existing prices to use as inputs and extend. This is expected to include p
+///   prices from all markets upstream of the markets we are calculating for.
 /// * `year` - The year for which prices are being calculated
 /// * `markets_to_price` - Set of markets to calculate full cost prices for
 /// * `commodities` - Map of all commodities (used to look up each commodity's `time_slice_level`)
 /// * `time_slice_info` - Time slice information (used to expand groups to individual time slices)
-///
-/// # Returns
-///
-/// A map of full cost prices for the specified markets in all time slices
 #[allow(clippy::too_many_arguments, clippy::too_many_lines)]
-fn calculate_full_cost_prices<'a, I, J>(
+fn add_full_cost_prices<'a, I, J>(
     activity_for_existing: I,
     activity_keys_for_candidates: J,
     annual_activities: &HashMap<AssetRef, Activity>,
-    upstream_prices: &CommodityPrices,
+    existing_prices: &mut CommodityPrices,
     year: u32,
     markets_to_price: &HashSet<(CommodityID, RegionID)>,
     commodities: &CommodityMap,
     time_slice_info: &TimeSliceInfo,
-) -> IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>
-where
+) where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
     J: Iterator<Item = (&'a AssetRef, &'a TimeSliceID)>,
 {
@@ -719,7 +693,7 @@ where
 
         // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
+            existing_prices,
             year,
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
@@ -779,7 +753,7 @@ where
 
         // Iterate over the marginal costs for commodities we need prices for
         for (commodity_id, marginal_cost) in asset.iter_marginal_costs_with_filter(
-            upstream_prices,
+            existing_prices,
             year,
             time_slice,
             |cid: &CommodityID| markets_to_price.contains(&(cid.clone(), region_id.clone())),
@@ -830,13 +804,12 @@ where
             .map(|v| (key, v))
     });
 
-    // Merge existing and candidate group prices, then expand to individual time slices
+    // Merge existing and candidate group prices
     let mut all_group_prices = group_prices;
     all_group_prices.extend(cand_group_prices);
 
-    let mut prices = IndexMap::new();
-    extend_selection_prices(&mut prices, &all_group_prices, time_slice_info);
-    prices
+    // Expand selection-level prices to individual time slices and add to the main prices map
+    extend_selection_prices(existing_prices, &all_group_prices, time_slice_info);
 }
 
 #[cfg(test)]
@@ -920,14 +893,14 @@ mod tests {
     }
 
     fn assert_price_approx(
-        prices: &IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>,
+        prices: &CommodityPrices,
         commodity: &CommodityID,
         region: &RegionID,
         time_slice: &TimeSliceID,
         expected: MoneyPerFlow,
     ) {
-        let p = prices[&(commodity.clone(), region.clone(), time_slice.clone())];
-        assert!((p - expected).abs() < MoneyPerFlow::EPSILON);
+        let p = prices.get(commodity, region, time_slice).unwrap();
+        assert_approx_eq!(MoneyPerFlow, p, expected);
     }
 
     #[rstest]
@@ -1038,10 +1011,11 @@ mod tests {
         let existing = vec![(&asset_ref, &time_slice, Activity(1.0))];
         let candidates = Vec::new();
 
-        let prices = calculate_marginal_cost_prices(
+        let mut prices = shadow_prices.clone();
+        add_marginal_cost_prices(
             existing.into_iter(),
             candidates.into_iter(),
-            &shadow_prices,
+            &mut prices,
             2015u32,
             &markets,
             &commodities,
@@ -1123,11 +1097,12 @@ mod tests {
         let mut annual_activities = HashMap::new();
         annual_activities.insert(asset_ref.clone(), Activity(2.0));
 
-        let prices = calculate_full_cost_prices(
+        let mut prices = shadow_prices.clone();
+        add_full_cost_prices(
             existing.into_iter(),
             candidates.into_iter(),
             &annual_activities,
-            &shadow_prices,
+            &mut prices,
             2015u32,
             &markets,
             &commodities,

From 68ba61ba696bac340a706a4643d32ae02e30cf09 Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Wed, 18 Mar 2026 11:33:37 +0000
Subject: [PATCH 19/23] Make extend_selection_prices a method of
 CommodityPrices

---
 src/simulation/prices.rs | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index a4ab5d0b0..4d09dc595 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -199,6 +199,20 @@ impl CommodityPrices {
         }
     }
 
+    /// Extend this map by applying each selection-level price to all time slices
+    /// contained in that selection.
+    fn extend_selection_prices(
+        &mut self,
+        group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
+        time_slice_info: &TimeSliceInfo,
+    ) {
+        for ((commodity_id, region_id, selection), &selection_price) in group_prices {
+            for (time_slice_id, _) in selection.iter(time_slice_info) {
+                self.insert(commodity_id, region_id, time_slice_id, selection_price);
+            }
+        }
+    }
+
     /// Iterate over the map.
     ///
     /// # Returns
@@ -377,20 +391,6 @@ fn add_scarcity_adjusted_prices<'a, I>(
     }
 }
 
-/// Extend an existing commodity/region/time-slice price map by applying each
-/// selection-level price to all time slices within that selection.
-fn extend_selection_prices(
-    prices: &mut CommodityPrices,
-    group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,
-    time_slice_info: &TimeSliceInfo,
-) {
-    for ((commodity_id, region_id, selection), &selection_price) in group_prices {
-        for (time_slice_id, _) in selection.iter(time_slice_info) {
-            prices.insert(commodity_id, region_id, time_slice_id, selection_price);
-        }
-    }
-}
-
 /// Calculate marginal cost prices for a set of commodities and add to an existing prices map.
 ///
 /// This pricing strategy aims to incorporate the marginal cost of commodity production into the price.
@@ -572,7 +572,7 @@ fn add_marginal_cost_prices<'a, I, J>(
     all_group_prices.extend(cand_group_prices);
 
     // Expand selection-level prices to individual time slices and add to the main prices map
-    extend_selection_prices(existing_prices, &all_group_prices, time_slice_info);
+    existing_prices.extend_selection_prices(&all_group_prices, time_slice_info);
 }
 
 /// Calculate annual activities for each asset by summing across all time slices
@@ -809,7 +809,7 @@ fn add_full_cost_prices<'a, I, J>(
     all_group_prices.extend(cand_group_prices);
 
     // Expand selection-level prices to individual time slices and add to the main prices map
-    extend_selection_prices(existing_prices, &all_group_prices, time_slice_info);
+    existing_prices.extend_selection_prices(&all_group_prices, time_slice_info);
 }
 
 #[cfg(test)]

From d9698da8e78937610ae683102541b81f9df1c96c Mon Sep 17 00:00:00 2001
From: Tom Bland <tom_bland@hotmail.co.uk>
Date: Wed, 18 Mar 2026 11:40:50 +0000
Subject: [PATCH 20/23] Use try_insert from input module

---
 src/simulation/prices.rs | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 4d09dc595..f7c2c7539 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -1,6 +1,7 @@
 //! Code for calculating commodity prices used by the simulation.
 use crate::asset::AssetRef;
 use crate::commodity::{CommodityID, CommodityMap, PricingStrategy};
+use crate::input::try_insert;
 use crate::model::Model;
 use crate::region::RegionID;
 use crate::simulation::optimisation::Solution;
@@ -175,7 +176,9 @@ pub fn calculate_prices(model: &Model, solution: &Solution, year: u32) -> Result
 pub struct CommodityPrices(IndexMap<(CommodityID, RegionID, TimeSliceID), MoneyPerFlow>);
 
 impl CommodityPrices {
-    /// Insert a price for the given commodity, region and time slice
+    /// Insert a price for the given commodity, region and time slice.
+    ///
+    /// Panics if a price for the given key already exists.
     pub fn insert(
         &mut self,
         commodity_id: &CommodityID,
@@ -184,8 +187,7 @@ impl CommodityPrices {
         price: MoneyPerFlow,
     ) {
         let key = (commodity_id.clone(), region_id.clone(), time_slice.clone());
-        let existing = self.0.insert(key.clone(), price).is_some();
-        assert!(!existing, "Key {key:?} already exists in the map");
+        try_insert(&mut self.0, &key, price).unwrap();
     }
 
     /// Extend the prices map, panic if any key already exists
@@ -194,13 +196,14 @@ impl CommodityPrices {
         T: IntoIterator<Item = ((CommodityID, RegionID, TimeSliceID), MoneyPerFlow)>,
     {
         for (key, price) in iter {
-            let existing = self.0.insert(key.clone(), price).is_some();
-            assert!(!existing, "Key {key:?} already exists in the map");
+            try_insert(&mut self.0, &key, price).unwrap();
         }
     }
 
     /// Extend this map by applying each selection-level price to all time slices
     /// contained in that selection.
+    ///
+    /// Panics if any individual commodity/region/time slice key already exists in the map.
     fn extend_selection_prices(
         &mut self,
         group_prices: &IndexMap<(CommodityID, RegionID, TimeSliceSelection), MoneyPerFlow>,

From e49dbd79c61928a1519d22a3307e378599beae29 Mon Sep 17 00:00:00 2001
From: Tom Bland <t.bland@imperial.ac.uk>
Date: Wed, 18 Mar 2026 12:09:20 +0000
Subject: [PATCH 21/23] Fix typo

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 src/simulation/prices.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index f7c2c7539..c743c9de8 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -645,7 +645,7 @@ where
 /// * `activity_for_existing` - Iterator over `(asset, time_slice, activity)` from optimisation
 ///   solution for existing assets
 /// * `activity_keys_for_candidates` - Iterator over `(asset, time_slice)` for candidate assets
-/// * `existing_prices` - Existing prices to use as inputs and extend. This is expected to include p
+/// * `existing_prices` - Existing prices to use as inputs and extend. This is expected to include
 ///   prices from all markets upstream of the markets we are calculating for.
 /// * `year` - The year for which prices are being calculated
 /// * `markets_to_price` - Set of markets to calculate full cost prices for

From 076f3111aaef30034fc6ea6eec67485505108ea6 Mon Sep 17 00:00:00 2001
From: Tom Bland <t.bland@imperial.ac.uk>
Date: Wed, 18 Mar 2026 13:28:33 +0000
Subject: [PATCH 22/23] Spelling

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 src/simulation/prices.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index c743c9de8..8f75594f8 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -521,7 +521,7 @@ fn add_marginal_cost_prices<'a, I, J>(
         IndexMap<AssetRef, WeightedAverageAccumulator>,
     > = IndexMap::new();
 
-    // Iterate over candidate assets (assuming full utilization)
+    // Iterate over candidate assets (assuming full utilisation)
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();
 

From dd75c66ba808868bfe276bd0be3108942e26bb4f Mon Sep 17 00:00:00 2001
From: Tom Bland <t.bland@imperial.ac.uk>
Date: Wed, 18 Mar 2026 13:28:43 +0000
Subject: [PATCH 23/23] Spelling

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 src/simulation/prices.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/simulation/prices.rs b/src/simulation/prices.rs
index 8f75594f8..b7012602e 100644
--- a/src/simulation/prices.rs
+++ b/src/simulation/prices.rs
@@ -745,7 +745,7 @@ fn add_full_cost_prices<'a, I, J>(
         IndexMap<AssetRef, WeightedAverageAccumulator>,
     > = IndexMap::new();
 
-    // Iterate over candidate assets (assuming full utilization)
+    // Iterate over candidate assets (assuming full utilisation)
     for (asset, time_slice) in activity_keys_for_candidates {
         let region_id = asset.region_id();