Apply suggestions from @alexdewar

Author: tsmbland

src/graph/validate.rs

@@ -68,11 +68,11 @@ fn prepare_commodities_graph_for_validation(
     filtered_graph
 }
 
-/// Checks if a process can be active for a particular timeslice in a given year and region
+/// Checks if a process can be active for a particular time slice in a given year and region
 ///
 /// It considers all commission years that can lead to a running process in the target region and
 /// year, accounting for the process lifetime, and then checks if, for any of those, the process
-/// is active in the required timeslice. In other words, this checks if there is the _possibility_
+/// is active in the required time slice. In other words, this checks if there is the _possibility_
 /// of having an active process, although there is no guarantee of that happening since it depends
 /// on the investment.
 fn can_be_active(

src/process.rs

@@ -290,7 +290,7 @@ impl ActivityLimits {
         let lower = *ts_limit.start();
         let mut upper = *ts_limit.end();
 
-        // If there's a seasonal/annual limit, we must cap the timeslice limit to ensure that it
+        // If there's a seasonal/annual limit, we must cap the time slice limit to ensure that it
         // doesn't exceed the upper bound of the season/year
         if let Some(seasonal_limit) = self.seasonal_limits.get(&time_slice.season) {
             upper = upper.min(*seasonal_limit.end());
@@ -356,7 +356,7 @@ impl ActivityLimits {
 
     /// Iterate over all limits
     ///
-    /// This first iterates over all individual timeslice limits, followed by seasonal limits (if
+    /// This first iterates over all individual time slice limits, followed by seasonal limits (if
     /// any), and finally the annual limit (if any).
     pub fn iter_limits(
         &self,
@@ -1030,10 +1030,10 @@ mod tests {
     fn new_with_full_availability(time_slice_info2: TimeSliceInfo) {
         let limits = ActivityLimits::new_with_full_availability(&time_slice_info2);
 
-        // Each timeslice from the info should be present in the limits
+        // Each time slice from the info should be present in the limits
         for (ts_id, ts_len) in time_slice_info2.iter() {
             let l = limits.get_limit_for_time_slice(ts_id);
-            // Lower bound should be zero and upper bound equal to timeslice length
+            // Lower bound should be zero and upper bound equal to time slice length
             assert_eq!(*l.start(), Dimensionless(0.0));
             assert_eq!(*l.end(), Dimensionless(ts_len.value()));
         }
@@ -1048,15 +1048,15 @@ mod tests {
     fn new_from_limits_with_seasonal_limit_applied(time_slice_info2: TimeSliceInfo) {
         let mut limits = HashMap::new();
 
-        // Set a seasonal upper limit that is stricter than the sum of timeslices
+        // Set a seasonal upper limit that is stricter than the sum of time slices
         limits.insert(
             TimeSliceSelection::Season("winter".into()),
             Dimensionless(0.0)..=Dimensionless(0.01),
         );
 
         let result = ActivityLimits::new_from_limits(&limits, &time_slice_info2).unwrap();
 
-        // Each timeslice upper bound should be capped by the seasonal upper bound (0.01)
+        // Each time slice upper bound should be capped by the seasonal upper bound (0.01)
         for (ts_id, _ts_len) in time_slice_info2.iter() {
             let ts_limit = result.get_limit_for_time_slice(ts_id);
             assert_eq!(*ts_limit.end(), Dimensionless(0.01));
@@ -1071,15 +1071,15 @@ mod tests {
     fn new_from_limits_with_annual_limit_applied(time_slice_info2: TimeSliceInfo) {
         let mut limits = HashMap::new();
 
-        // Set an annual upper limit that is stricter than the sum of timeslices
+        // Set an annual upper limit that is stricter than the sum of time slices
         limits.insert(
             TimeSliceSelection::Annual,
             Dimensionless(0.0)..=Dimensionless(0.01),
         );
 
         let result = ActivityLimits::new_from_limits(&limits, &time_slice_info2).unwrap();
 
-        // Each timeslice upper bound should be capped by the annual upper bound (0.01)
+        // Each time slice upper bound should be capped by the annual upper bound (0.01)
         for (ts_id, _ts_len) in time_slice_info2.iter() {
             let ts_limit = result.get_limit_for_time_slice(ts_id);
             assert_eq!(*ts_limit.end(), Dimensionless(0.01));
@@ -1098,7 +1098,7 @@ mod tests {
     fn new_from_limits_missing_timeslices_error(time_slice_info2: TimeSliceInfo) {
         let mut limits = HashMap::new();
 
-        // Add a single timeslice limit but do not provide limits for all timeslices
+        // Add a single time slice limit but do not provide limits for all time slices
         let first_ts = time_slice_info2.iter().next().unwrap().0.clone();
         limits.insert(
             TimeSliceSelection::Single(first_ts),

src/simulation/prices.rs

@@ -508,7 +508,7 @@ fn add_marginal_cost_prices<'a, I, J>(
         year,
         commodities,
         &PricingStrategy::MarginalCost,
-        None::<&HashMap<AssetRef, Activity>>,
+        /*annual_activities=*/ None,
     )
     .collect();
     let priced_groups: HashSet<_> = group_prices.keys().cloned().collect();
@@ -567,10 +567,17 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
 {
     // Validate supported strategies, and require annual activities for FullCost pricing.
-    assert!(matches!(
-        (pricing_strategy, annual_activities),
-        (PricingStrategy::MarginalCost, _) | (PricingStrategy::FullCost, Some(_))
-    ));
+    match pricing_strategy {
+        PricingStrategy::MarginalCost => assert!(
+            annual_activities.is_none(),
+            "Cannot provide annual_activities with marginal pricing strategy"
+        ),
+        PricingStrategy::FullCost => assert!(
+            annual_activities.is_some(),
+            "annual_activities must be provided for full pricing strategy"
+        ),
+        _ => panic!("Invalid pricing strategy"),
+    }
 
     // Accumulator map to collect costs from existing assets. For each (commodity, region,
     // ts selection), this maps each asset to a weighted average of the costs for that
@@ -583,16 +590,15 @@ where
     > = IndexMap::new();
 
     // Cache of annual fixed costs per flow for each asset (only used for Full cost pricing)
-    let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
+    let mut annual_fixed_costs = HashMap::new();
 
     // Iterate over existing assets and their activities
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
 
         // When using full cost pricing, skip assets with zero activity across the year, since
         // we cannot calculate a fixed cost per flow.
-        let annual_activity = matches!(pricing_strategy, PricingStrategy::FullCost)
-            .then(|| annual_activities.unwrap()[asset]);
+        let annual_activity = annual_activities.map(|activities| activities[asset]);
         if annual_activity.is_some_and(|annual_activity| annual_activity < Activity::EPSILON) {
             continue;
         }
@@ -654,7 +660,7 @@ where
 /// involves taking a weighted average across time slices for each asset according to potential
 /// activity (i.e. the upper activity limit), omitting prices in the extreme case of zero potential
 /// activity (Note: this should NOT happen as validation should ensure there is at least one
-/// candidate that can provide a price in each timeslice for which a price could be required).
+/// candidate that can provide a price in each time slice for which a price could be required).
 /// Costs for candidates are calculated assuming full utilisation.
 ///
 /// # Arguments
@@ -692,10 +698,10 @@ where
     ));
 
     // Cache of annual fixed costs per flow for each asset (only used for Full cost pricing)
-    let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
+    let mut annual_fixed_costs = HashMap::new();
 
     // Cache of annual activity limits for each asset (only used for Full cost pricing)
-    let mut annual_activity_limits: HashMap<_, _> = HashMap::new();
+    let mut annual_activity_limits = HashMap::new();
 
     // Accumulator map to collect costs from candidate assets. Similar to existing_accum,
     // but costs are weighted according to activity limits (i.e. assuming full utilisation).
@@ -813,7 +819,7 @@ fn add_marginal_cost_average_prices<'a, I, J>(
         year,
         commodities,
         &PricingStrategy::MarginalCost,
-        None::<&HashMap<AssetRef, Activity>>,
+        /*annual_activities=*/ None,
     )
     .collect();
     let priced_groups: HashSet<_> = group_prices.keys().cloned().collect();
@@ -872,10 +878,17 @@ where
     I: Iterator<Item = (&'a AssetRef, &'a TimeSliceID, Activity)>,
 {
     // Validate supported strategies, and require annual activities for FullCost pricing.
-    assert!(matches!(
-        (pricing_strategy, annual_activities),
-        (PricingStrategy::MarginalCost, _) | (PricingStrategy::FullCost, Some(_))
-    ));
+    match pricing_strategy {
+        PricingStrategy::MarginalCost => assert!(
+            annual_activities.is_none(),
+            "Cannot provide annual_activities with marginal pricing strategy"
+        ),
+        PricingStrategy::FullCost => assert!(
+            annual_activities.is_some(),
+            "annual_activities must be provided for full pricing strategy"
+        ),
+        _ => panic!("Invalid pricing strategy"),
+    }
 
     // Accumulator map to collect costs from existing assets. Collects a weighted average
     // for each (commodity, region, ts selection), across all contributing assets, weighted
@@ -888,16 +901,15 @@ where
     > = IndexMap::new();
 
     // Cache of annual fixed costs per flow for each asset (only used for Full cost pricing)
-    let mut annual_fixed_costs: HashMap<_, _> = HashMap::new();
+    let mut annual_fixed_costs = HashMap::new();
 
     // Iterate over existing assets and their activities
     for (asset, time_slice, activity) in activity_for_existing {
         let region_id = asset.region_id();
 
         // When using full cost pricing, skip assets with zero annual activity, since we cannot
         // calculate a fixed cost per flow.
-        let annual_activity = matches!(pricing_strategy, PricingStrategy::FullCost)
-            .then(|| annual_activities.unwrap()[asset]);
+        let annual_activity = annual_activities.map(|activities| activities[asset]);
         if annual_activity.is_some_and(|annual_activity| annual_activity < Activity::EPSILON) {
             continue;
         }