From b89a63a079004b372a72393606c61eff9e2da67b Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Thu, 24 Apr 2025 09:41:24 +0100 Subject: [PATCH 1/8] Add region_id to process_parameters table --- examples/simple/process_parameters.csv | 14 ++++---- src/asset.rs | 4 +-- src/input/process.rs | 9 +++-- src/input/process/parameter.rs | 49 ++++++++++++++++++++------ src/process.rs | 4 +-- src/region.rs | 36 +++++++++++++++++++ src/simulation/optimisation.rs | 2 +- src/utils.rs | 4 +-- 8 files changed, 96 insertions(+), 26 deletions(-) diff --git a/examples/simple/process_parameters.csv b/examples/simple/process_parameters.csv index 283a0913b..c61f51cfe 100644 --- a/examples/simple/process_parameters.csv +++ b/examples/simple/process_parameters.csv @@ -1,7 +1,7 @@ -process_id,capital_cost,fixed_operating_cost,variable_operating_cost,lifetime,discount_rate,capacity_to_activity,year -GASDRV,10.0,0.3,2.0,25,0.1,1.0,all -GASPRC,7.0,0.21,0.5,25,0.1,1.0,all -WNDFRM,1000.0,30.0,0.4,25,0.1,31.54,all -GASCGT,700.0,21.0,0.55,30,0.1,31.54,all -RGASBR,55.56,1.6668,0.16,15,0.1,1.0,all -RELCHP,138.9,4.167,0.17,15,0.1,1.0,all +process_id,capital_cost,fixed_operating_cost,variable_operating_cost,lifetime,discount_rate,capacity_to_activity,year,region_id +GASDRV,10.0,0.3,2.0,25,0.1,1.0,all,GBR +GASPRC,7.0,0.21,0.5,25,0.1,1.0,all,GBR +WNDFRM,1000.0,30.0,0.4,25,0.1,31.54,all,GBR +GASCGT,700.0,21.0,0.55,30,0.1,31.54,all,GBR +RGASBR,55.56,1.6668,0.16,15,0.1,1.0,all,GBR +RELCHP,138.9,4.167,0.17,15,0.1,1.0,all,GBR diff --git a/src/asset.rs b/src/asset.rs index a2ecf6a88..d3d0f120e 100644 --- a/src/asset.rs +++ b/src/asset.rs @@ -62,7 +62,7 @@ impl Asset { + self .process .parameter - .get(&self.commission_year) + .get(&(self.region_id.clone(), self.commission_year)) .unwrap() .lifetime } @@ -84,7 +84,7 @@ impl Asset { * self .process .parameter - .get(&self.commission_year) + .get(&(self.region_id.clone(), self.commission_year)) .unwrap() .capacity_to_activity } diff --git a/src/input/process.rs b/src/input/process.rs index d259d0963..0b9354d2f 100644 --- a/src/input/process.rs +++ b/src/input/process.rs @@ -60,8 +60,13 @@ pub fn read_processes( let mut availabilities = read_process_availabilities(model_dir, &process_ids, time_slice_info)?; let mut flows = read_process_flows(model_dir, &process_ids, commodities)?; - let mut parameters = - read_process_parameters(model_dir, &process_ids, &processes, milestone_years)?; + let mut parameters = read_process_parameters( + model_dir, + &process_ids, + &processes, + milestone_years, + region_ids, + )?; let mut regions = read_process_regions(model_dir, &process_ids, region_ids)?; // Validate commodities after the flows have been read diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index dfde564b3..6aa2b8a8f 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -2,6 +2,7 @@ use super::super::*; use crate::id::IDCollection; use crate::process::{Process, ProcessID, ProcessParameter, ProcessParameterMap}; +use crate::region::{deserialize_region, RegionID, RegionSelection}; use crate::utils::try_insert; use crate::year::{deserialize_year, YearSelection}; use ::log::warn; @@ -23,6 +24,8 @@ struct ProcessParameterRaw { capacity_to_activity: Option, #[serde(deserialize_with = "deserialize_year")] year: YearSelection, + #[serde(deserialize_with = "deserialize_region")] + region: RegionSelection, } impl ProcessParameterRaw { @@ -98,10 +101,11 @@ pub fn read_process_parameters( process_ids: &HashSet, processes: &HashMap, milestone_years: &[u32], + region_ids: &HashSet, ) -> Result> { let file_path = model_dir.join(PROCESS_PARAMETERS_FILE_NAME); let iter = read_csv::(&file_path)?; - read_process_parameters_from_iter(iter, process_ids, processes, milestone_years) + read_process_parameters_from_iter(iter, process_ids, processes, milestone_years, region_ids) .with_context(|| input_err_msg(&file_path)) } @@ -110,6 +114,7 @@ fn read_process_parameters_from_iter( process_ids: &HashSet, processes: &HashMap, milestone_years: &[u32], + region_ids: &HashSet, ) -> Result> where I: Iterator, @@ -118,6 +123,7 @@ where for param_raw in iter { let id = process_ids.get_id_by_str(¶m_raw.process_id)?; let year = param_raw.year.clone(); + let region = param_raw.region.clone(); let param = param_raw.into_parameter()?; let entry = params.entry(id.clone()).or_default(); @@ -126,23 +132,45 @@ where .ok_or_else(|| anyhow::anyhow!("Process {} not found", id))?; let year_range = process.years.clone(); - match year { - YearSelection::Some(years) => { - for year in years { - try_insert(entry, year, param.clone())?; + match (region, year) { + (RegionSelection::Some(regions), YearSelection::Some(years)) => { + for region in regions { + for year in years.clone() { + try_insert(entry, (region.clone(), year), param.clone())?; + } + } + } + (RegionSelection::Some(regions), YearSelection::All) => { + for region in regions { + for year in milestone_years.iter() { + if year_range.contains(year) { + try_insert(entry, (region.clone(), *year), param.clone())?; + } + } + } + } + (RegionSelection::All, YearSelection::Some(years)) => { + // NOTE: This iterates over ALL regions, not just the ones applicable to the + // process - we should change this. + for region in region_ids.iter() { + for year in years.clone() { + try_insert(entry, (region.clone(), year), param.clone())?; + } } } - YearSelection::All => { - for year in milestone_years.iter() { - if year_range.contains(year) { - try_insert(entry, *year, param.clone())?; + (RegionSelection::All, YearSelection::All) => { + for region in region_ids.iter() { + for year in milestone_years.iter() { + if year_range.contains(year) { + try_insert(entry, (region.clone(), *year), param.clone())?; + } } } } } } - // Check parameters cover all years of the process + // Check parameters cover all years and of the process and all regions for (id, parameter) in params.iter() { let year_range = processes.get(id).unwrap().years.clone(); let reference_years: HashSet = milestone_years @@ -179,6 +207,7 @@ mod tests { discount_rate, capacity_to_activity, year: YearSelection::All, + region: RegionSelection::All, } } diff --git a/src/process.rs b/src/process.rs index 0d04ad2cc..bf503ccde 100644 --- a/src/process.rs +++ b/src/process.rs @@ -2,7 +2,7 @@ //! module are used to represent these conversions along with the associated costs. use crate::commodity::Commodity; use crate::id::define_id_type; -use crate::region::RegionSelection; +use crate::region::{RegionID, RegionSelection}; use crate::time_slice::TimeSliceID; use indexmap::IndexMap; use serde::Deserialize; @@ -61,7 +61,7 @@ impl Process { pub type EnergyLimitsMap = HashMap>; /// A map of [`ProcessParameter`]s, keyed by year -pub type ProcessParameterMap = HashMap; +pub type ProcessParameterMap = HashMap<(RegionID, u32), ProcessParameter>; /// Represents a maximum annual commodity flow for a given process #[derive(PartialEq, Debug, Deserialize, Clone)] diff --git a/src/region.rs b/src/region.rs index ea7d9d7c2..8bf229ee0 100644 --- a/src/region.rs +++ b/src/region.rs @@ -3,12 +3,22 @@ use crate::id::define_id_getter; use crate::id::define_id_type; use indexmap::IndexMap; use itertools::Itertools; +use serde::de::Deserializer; use serde::Deserialize; use std::collections::HashSet; use std::fmt::Display; +use std::str::FromStr; define_id_type! {RegionID} +impl FromStr for RegionID { + type Err = String; + + fn from_str(s: &str) -> Result { + Ok(RegionID::from(s)) + } +} + /// A map of [`Region`]s, keyed by region ID pub type RegionMap = IndexMap; @@ -50,3 +60,29 @@ impl Display for RegionSelection { } } } + +/// Deserialises a region selection from a string. The string can be either "all", a single region, or a +/// semicolon-separated list of regions (e.g. "GBR;FRA;ESP" or "GBR; FRA; ESP"). +pub fn deserialize_region<'de, D>(deserialiser: D) -> Result +where + D: Deserializer<'de>, +{ + let value = String::deserialize(deserialiser)?; + if value.trim().eq_ignore_ascii_case("all") { + // "all" regions specified + Ok(RegionSelection::All) + } else { + // Semicolon-separated list of regions + let regions: Result, _> = value + .split(';') + .map(|s| s.trim().parse::()) + .collect(); + match regions { + Ok(regions_set) if !regions_set.is_empty() => Ok(RegionSelection::Some(regions_set)), + _ => Err(serde::de::Error::custom(format!( + "Invalid region format: {}", + value + ))), + } + } +} diff --git a/src/simulation/optimisation.rs b/src/simulation/optimisation.rs index 634d7fd39..10462bc27 100644 --- a/src/simulation/optimisation.rs +++ b/src/simulation/optimisation.rs @@ -231,7 +231,7 @@ fn calculate_cost_coefficient( coeff += asset .process .parameter - .get(&asset.commission_year) + .get(&(asset.region_id.clone(), asset.commission_year)) .unwrap() .variable_operating_cost } diff --git a/src/utils.rs b/src/utils.rs index cbfd97165..61d4ad361 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -9,13 +9,13 @@ use std::hash::Hash; /// If the key already exists, it returns an error with a message indicating the key's existence. pub fn try_insert(map: &mut HashMap, key: K, value: V) -> Result<()> where - K: Eq + Hash + std::fmt::Display, + K: Eq + Hash + std::fmt::Debug, { match map.entry(key) { Vacant(entry) => { entry.insert(value); Ok(()) } - Occupied(entry) => Err(anyhow!("Key {} already exists in the map", entry.key())), + Occupied(entry) => Err(anyhow!("Key {:?} already exists in the map", entry.key())), } } From 1f80aa2dbeb9ff332b08f33e766650932d86cd6e Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Thu, 24 Apr 2025 10:09:56 +0100 Subject: [PATCH 2/8] Fix remaining issues with main code --- src/input/process.rs | 7 ++-- src/input/process/parameter.rs | 61 ++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/src/input/process.rs b/src/input/process.rs index 0b9354d2f..32ec47d14 100644 --- a/src/input/process.rs +++ b/src/input/process.rs @@ -60,14 +60,15 @@ pub fn read_processes( let mut availabilities = read_process_availabilities(model_dir, &process_ids, time_slice_info)?; let mut flows = read_process_flows(model_dir, &process_ids, commodities)?; + let mut process_regions = read_process_regions(model_dir, &process_ids, region_ids)?; let mut parameters = read_process_parameters( model_dir, &process_ids, &processes, milestone_years, region_ids, + &process_regions, )?; - let mut regions = read_process_regions(model_dir, &process_ids, region_ids)?; // Validate commodities after the flows have been read validate_commodities( @@ -85,7 +86,7 @@ pub fn read_processes( process.energy_limits = availabilities.remove(id).unwrap(); process.flows = flows.remove(id).unwrap(); process.parameter = parameters.remove(id).unwrap(); - process.regions = regions.remove(id).unwrap(); + process.regions = process_regions.remove(id).unwrap(); } // Create ProcessMap @@ -240,7 +241,7 @@ fn validate_svd_commodity( .get(&*flow.process_id) .unwrap() .keys() - .contains(&year) + .contains(&(region_id.clone(), year)) && params .availabilities .get(&*flow.process_id) diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index 6aa2b8a8f..4605650ca 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -102,11 +102,19 @@ pub fn read_process_parameters( processes: &HashMap, milestone_years: &[u32], region_ids: &HashSet, + process_regions: &HashMap, ) -> Result> { let file_path = model_dir.join(PROCESS_PARAMETERS_FILE_NAME); let iter = read_csv::(&file_path)?; - read_process_parameters_from_iter(iter, process_ids, processes, milestone_years, region_ids) - .with_context(|| input_err_msg(&file_path)) + read_process_parameters_from_iter( + iter, + process_ids, + processes, + milestone_years, + region_ids, + process_regions, + ) + .with_context(|| input_err_msg(&file_path)) } fn read_process_parameters_from_iter( @@ -115,6 +123,7 @@ fn read_process_parameters_from_iter( processes: &HashMap, milestone_years: &[u32], region_ids: &HashSet, + process_regions: &HashMap, ) -> Result> where I: Iterator, @@ -131,6 +140,9 @@ where .get(&id) .ok_or_else(|| anyhow::anyhow!("Process {} not found", id))?; let year_range = process.years.clone(); + let process_regions = process_regions + .get(&id) + .ok_or_else(|| anyhow::anyhow!("Regions not found for process {}", id))?; match (region, year) { (RegionSelection::Some(regions), YearSelection::Some(years)) => { @@ -150,19 +162,21 @@ where } } (RegionSelection::All, YearSelection::Some(years)) => { - // NOTE: This iterates over ALL regions, not just the ones applicable to the - // process - we should change this. for region in region_ids.iter() { - for year in years.clone() { - try_insert(entry, (region.clone(), year), param.clone())?; + if process_regions.contains(region) { + for year in years.clone() { + try_insert(entry, (region.clone(), year), param.clone())?; + } } } } (RegionSelection::All, YearSelection::All) => { for region in region_ids.iter() { - for year in milestone_years.iter() { - if year_range.contains(year) { - try_insert(entry, (region.clone(), *year), param.clone())?; + if process_regions.contains(region) { + for year in milestone_years.iter() { + if year_range.contains(year) { + try_insert(entry, (region.clone(), *year), param.clone())?; + } } } } @@ -170,7 +184,7 @@ where } } - // Check parameters cover all years and of the process and all regions + // Check parameters cover all years and regions of the process for (id, parameter) in params.iter() { let year_range = processes.get(id).unwrap().years.clone(); let reference_years: HashSet = milestone_years @@ -178,14 +192,27 @@ where .copied() .filter(|year| year_range.contains(year)) .collect(); - let parameter_years: HashSet = parameter.keys().copied().collect(); - ensure!( - parameter_years == reference_years, - "Error in parameters for process {}: years do not match the process years", - id - ); + let region_selection = process_regions.get(id).unwrap().clone(); + let reference_regions: HashSet = match region_selection { + RegionSelection::All => region_ids.clone(), + RegionSelection::Some(ref regions) => regions.clone(), + }; + let mut missing_keys = Vec::new(); + for year in reference_years.iter() { + for region in reference_regions.iter() { + if !parameter.contains_key(&(region.clone(), *year)) { + missing_keys.push((region, *year)); + } + } + } + if !missing_keys.is_empty() { + return Err(anyhow::anyhow!( + "Process {} is missing parameters for the following regions and years: {:?}", + id, + missing_keys + )); + } } - Ok(params) } From bd3e56d16d75bde784f23ff1224d1818778efeaa Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Thu, 24 Apr 2025 10:14:13 +0100 Subject: [PATCH 3/8] Fix tests --- src/asset.rs | 4 ++-- src/input/process.rs | 2 +- src/simulation/optimisation.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/asset.rs b/src/asset.rs index d3d0f120e..220509b84 100644 --- a/src/asset.rs +++ b/src/asset.rs @@ -226,7 +226,7 @@ mod tests { let years = RangeInclusive::new(2010, 2020).collect_vec(); let process_parameter_map: ProcessParameterMap = years .iter() - .map(|&year| (year, process_param.clone())) + .map(|&year| (("GBR".into(), year), process_param.clone())) .collect(); let commodity = Rc::new(Commodity { id: "commodity1".into(), @@ -279,7 +279,7 @@ mod tests { let years = RangeInclusive::new(2010, 2020).collect_vec(); let process_parameter_map: ProcessParameterMap = years .iter() - .map(|&year| (year, process_param.clone())) + .map(|&year| (("GBR".into(), year), process_param.clone())) .collect(); let process = Rc::new(Process { id: "process1".into(), diff --git a/src/input/process.rs b/src/input/process.rs index 32ec47d14..215bf12a7 100644 --- a/src/input/process.rs +++ b/src/input/process.rs @@ -317,7 +317,7 @@ mod tests { capacity_to_activity: 0.0, }; for year in [2010, 2020] { - parameter_map.insert(year, parameter.clone()); + parameter_map.insert(("GBR".into(), year), parameter.clone()); } (id.into(), parameter_map) }) diff --git a/src/simulation/optimisation.rs b/src/simulation/optimisation.rs index 10462bc27..a73470b1a 100644 --- a/src/simulation/optimisation.rs +++ b/src/simulation/optimisation.rs @@ -289,8 +289,8 @@ mod tests { capacity_to_activity: 1.0, }; let mut process_parameter_map = ProcessParameterMap::new(); - process_parameter_map.insert(2010, process_param.clone()); - process_parameter_map.insert(2020, process_param.clone()); + process_parameter_map.insert(("GBR".into(), 2010), process_param.clone()); + process_parameter_map.insert(("GBR".into(), 2020), process_param.clone()); let commodity = Rc::new(Commodity { id: "commodity1".into(), description: "Some description".into(), From dda70bf3be57ea23a63e879f1de7ead02643fb05 Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Thu, 24 Apr 2025 10:45:06 +0100 Subject: [PATCH 4/8] Small tidy-ups --- src/input/process/parameter.rs | 69 +++++++++++++--------------------- 1 file changed, 26 insertions(+), 43 deletions(-) diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index 4605650ca..0d636046c 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -144,62 +144,45 @@ where .get(&id) .ok_or_else(|| anyhow::anyhow!("Regions not found for process {}", id))?; - match (region, year) { - (RegionSelection::Some(regions), YearSelection::Some(years)) => { - for region in regions { - for year in years.clone() { - try_insert(entry, (region.clone(), year), param.clone())?; - } - } - } - (RegionSelection::Some(regions), YearSelection::All) => { - for region in regions { - for year in milestone_years.iter() { - if year_range.contains(year) { - try_insert(entry, (region.clone(), *year), param.clone())?; - } - } - } - } - (RegionSelection::All, YearSelection::Some(years)) => { - for region in region_ids.iter() { - if process_regions.contains(region) { - for year in years.clone() { - try_insert(entry, (region.clone(), year), param.clone())?; - } - } - } - } - (RegionSelection::All, YearSelection::All) => { - for region in region_ids.iter() { - if process_regions.contains(region) { - for year in milestone_years.iter() { - if year_range.contains(year) { - try_insert(entry, (region.clone(), *year), param.clone())?; - } - } - } - } + let selected_regions: Vec<_> = match region { + RegionSelection::Some(regions) => regions.iter().cloned().collect(), + RegionSelection::All => region_ids + .iter() + .filter(|r| process_regions.contains(r)) + .cloned() + .collect(), + }; + let selected_years: Vec<_> = match year { + YearSelection::Some(years) => years.iter().cloned().collect(), + YearSelection::All => milestone_years + .iter() + .cloned() + .filter(|y| year_range.contains(y)) + .collect(), + }; + for region in selected_regions { + for year in &selected_years { + try_insert(entry, (region.clone(), *year), param.clone())?; } } } // Check parameters cover all years and regions of the process for (id, parameter) in params.iter() { - let year_range = processes.get(id).unwrap().years.clone(); + let year_range = &processes.get(id).unwrap().years; let reference_years: HashSet = milestone_years .iter() .copied() .filter(|year| year_range.contains(year)) .collect(); - let region_selection = process_regions.get(id).unwrap().clone(); - let reference_regions: HashSet = match region_selection { - RegionSelection::All => region_ids.clone(), - RegionSelection::Some(ref regions) => regions.clone(), + let reference_regions = match process_regions.get(id).unwrap() { + RegionSelection::All => region_ids, + RegionSelection::Some(ref regions) => regions, }; + let mut missing_keys = Vec::new(); - for year in reference_years.iter() { - for region in reference_regions.iter() { + for year in &reference_years { + for region in reference_regions { if !parameter.contains_key(&(region.clone(), *year)) { missing_keys.push((region, *year)); } From a594dbfe33f1a39f9b38bf1df3cc13c81ccf812e Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Thu, 24 Apr 2025 10:56:11 +0100 Subject: [PATCH 5/8] Fix naming issue, and check for missing parameters --- src/input/process/parameter.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index 0d636046c..aed8436e9 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -25,7 +25,7 @@ struct ProcessParameterRaw { #[serde(deserialize_with = "deserialize_year")] year: YearSelection, #[serde(deserialize_with = "deserialize_region")] - region: RegionSelection, + region_id: RegionSelection, } impl ProcessParameterRaw { @@ -132,7 +132,7 @@ where for param_raw in iter { let id = process_ids.get_id_by_str(¶m_raw.process_id)?; let year = param_raw.year.clone(); - let region = param_raw.region.clone(); + let region = param_raw.region_id.clone(); let param = param_raw.into_parameter()?; let entry = params.entry(id.clone()).or_default(); @@ -167,6 +167,13 @@ where } } + // Check if all processes are present in the parameters + for id in process_ids { + if !params.contains_key(id) { + return Err(anyhow::anyhow!("Process {} is missing parameters", id)); + } + } + // Check parameters cover all years and regions of the process for (id, parameter) in params.iter() { let year_range = &processes.get(id).unwrap().years; From a6229cbbd252b1b134b9f324c904cd12c410fb1e Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Thu, 24 Apr 2025 10:58:27 +0100 Subject: [PATCH 6/8] Fix broken test --- src/input/process/parameter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index aed8436e9..e236bd53b 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -224,7 +224,7 @@ mod tests { discount_rate, capacity_to_activity, year: YearSelection::All, - region: RegionSelection::All, + region_id: RegionSelection::All, } } From 1e63f7693f76715e762cc19d592fc67bc2ce7808 Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Tue, 29 Apr 2025 14:01:32 +0100 Subject: [PATCH 7/8] Tidy ups --- src/input/process/parameter.rs | 30 +++++++++++------------------- src/region.rs | 9 --------- 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index fb18868f1..ea8f54cb1 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -114,7 +114,7 @@ fn read_process_parameters_from_iter( where I: Iterator, { - let mut params: HashMap = HashMap::new(); + let mut map: HashMap = HashMap::new(); for param_raw in iter { // Get process let id = process_ids.get_id_by_str(¶m_raw.process_id)?; @@ -143,7 +143,7 @@ where // Insert parameter into the map let param = param_raw.into_parameter()?; - let entry = params.entry(id.clone()).or_default(); + let entry = map.entry(id.clone()).or_default(); for year in parameter_years { for region in parameter_regions.clone() { try_insert(entry, (region, year), param.clone())?; @@ -151,15 +151,8 @@ where } } - // Check if all processes are present in the parameters - for id in process_ids { - if !params.contains_key(id) { - return Err(anyhow::anyhow!("Process {} is missing parameters", id)); - } - } - // Check parameters cover all years and regions of the process - for (id, parameter) in params.iter() { + for (id, parameters) in map.iter() { let process = processes.get(id).unwrap(); let year_range = process.years.clone(); let reference_years: HashSet = milestone_years @@ -172,20 +165,19 @@ where let mut missing_keys = Vec::new(); for year in &reference_years { for region in &reference_regions { - if !parameter.contains_key(&(region.clone(), *year)) { + if !parameters.contains_key(&(region.clone(), *year)) { missing_keys.push((region, *year)); } } } - if !missing_keys.is_empty() { - return Err(anyhow::anyhow!( - "Process {} is missing parameters for the following regions and years: {:?}", - id, - missing_keys - )); - } + ensure!( + !missing_keys.is_empty(), + "Process {} is missing parameters for the following regions and years: {:?}", + id, + missing_keys + ); } - Ok(params) + Ok(map) } #[cfg(test)] diff --git a/src/region.rs b/src/region.rs index 7cfe44a04..0bcf9ba3f 100644 --- a/src/region.rs +++ b/src/region.rs @@ -4,18 +4,9 @@ use anyhow::{ensure, Result}; use indexmap::IndexMap; use serde::Deserialize; use std::collections::HashSet; -use std::str::FromStr; define_id_type! {RegionID} -impl FromStr for RegionID { - type Err = String; - - fn from_str(s: &str) -> Result { - Ok(RegionID::from(s)) - } -} - /// A map of [`Region`]s, keyed by region ID pub type RegionMap = IndexMap; From f075cfdee3420cdbd144bc044cdd8323d2b57d79 Mon Sep 17 00:00:00 2001 From: Tom Bland Date: Tue, 29 Apr 2025 16:36:27 +0100 Subject: [PATCH 8/8] Apply suggestions --- src/input/process/parameter.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/input/process/parameter.rs b/src/input/process/parameter.rs index ea8f54cb1..a4ee3c0a3 100644 --- a/src/input/process/parameter.rs +++ b/src/input/process/parameter.rs @@ -154,7 +154,7 @@ where // Check parameters cover all years and regions of the process for (id, parameters) in map.iter() { let process = processes.get(id).unwrap(); - let year_range = process.years.clone(); + let year_range = &process.years; let reference_years: HashSet = milestone_years .iter() .copied() @@ -165,8 +165,9 @@ where let mut missing_keys = Vec::new(); for year in &reference_years { for region in &reference_regions { - if !parameters.contains_key(&(region.clone(), *year)) { - missing_keys.push((region, *year)); + let key = (region.clone(), *year); + if !parameters.contains_key(&key) { + missing_keys.push(key); } } }