diff --git a/pywr-schema/src/metric.rs b/pywr-schema/src/metric.rs index 167279f1..07715959 100644 --- a/pywr-schema/src/metric.rs +++ b/pywr-schema/src/metric.rs @@ -211,7 +211,7 @@ impl TryFromV1 for Metric { Self::Parameter(reference) } - ParameterOrTimeseriesRef::Timeseries(t) => Self::Timeseries(t), + ParameterOrTimeseriesRef::Timeseries(t) => Self::Timeseries(t.ts_ref), } } }; diff --git a/pywr-schema/src/model.rs b/pywr-schema/src/model.rs index ea6b41af..2bbebcc3 100644 --- a/pywr-schema/src/model.rs +++ b/pywr-schema/src/model.rs @@ -317,11 +317,12 @@ impl PywrNetwork { timeseries.extend(conversion_data.timeseries); parameters.extend(conversion_data.parameters); - // closure to update a parameter ref with a timeseries ref when names match. + // Closure to update a parameter ref with a timeseries ref when names match. + // We match on the original parameter name because the parameter name may have been changed let update_to_ts_ref = &mut |m: &mut Metric| { if let Metric::Parameter(p) = m { - if let Some(ts_ref) = timeseries_refs.iter().find(|ts| ts.name() == p.name) { - *m = Metric::Timeseries(ts_ref.clone()); + if let Some(converted_ts_ref) = timeseries_refs.iter().find(|ts| ts.original_parameter_name == p.name) { + *m = Metric::Timeseries(converted_ts_ref.ts_ref.clone()); } } }; diff --git a/pywr-schema/src/parameters/mod.rs b/pywr-schema/src/parameters/mod.rs index 5f6b4226..31dafe09 100644 --- a/pywr-schema/src/parameters/mod.rs +++ b/pywr-schema/src/parameters/mod.rs @@ -32,7 +32,7 @@ use crate::error::SchemaError; use crate::metric::{Metric, ParameterReference}; #[cfg(feature = "core")] use crate::model::LoadArgs; -use crate::timeseries::TimeseriesReference; +use crate::timeseries::ConvertedTimeseriesReference; use crate::v1::{ConversionData, TryFromV1, TryIntoV2}; use crate::visit::{VisitMetrics, VisitPaths}; pub use aggregated::{AggFunc, AggregatedIndexParameter, AggregatedParameter, IndexAggFunc}; @@ -357,7 +357,7 @@ impl VisitPaths for Parameter { pub enum ParameterOrTimeseriesRef { // Boxed due to large size difference. Parameter(Box), - Timeseries(TimeseriesReference), + Timeseries(ConvertedTimeseriesReference), } impl From for ParameterOrTimeseriesRef { @@ -366,8 +366,8 @@ impl From for ParameterOrTimeseriesRef { } } -impl From for ParameterOrTimeseriesRef { - fn from(t: TimeseriesReference) -> Self { +impl From for ParameterOrTimeseriesRef { + fn from(t: ConvertedTimeseriesReference) -> Self { Self::Timeseries(t) } } @@ -430,12 +430,14 @@ impl TryFromV1 for ParameterOrTimeseriesRef { } CoreParameter::Min(p) => Parameter::Min(p.try_into_v2(parent_node, conversion_data)?).into(), CoreParameter::Division(p) => Parameter::Division(p.try_into_v2(parent_node, conversion_data)?).into(), - CoreParameter::DataFrame(p) => >::try_into_v2( - p, - parent_node, - conversion_data, - )? - .into(), + CoreParameter::DataFrame(p) => { + >::try_into_v2( + p, + parent_node, + conversion_data, + )? + .into() + } CoreParameter::Deficit(p) => { return Err(ConversionError::DeprecatedParameter { ty: "DeficitParameter".to_string(), diff --git a/pywr-schema/src/timeseries/mod.rs b/pywr-schema/src/timeseries/mod.rs index 244cac13..3fd49cda 100644 --- a/pywr-schema/src/timeseries/mod.rs +++ b/pywr-schema/src/timeseries/mod.rs @@ -319,7 +319,18 @@ impl TimeseriesReference { } } -impl TryFromV1 for TimeseriesReference { +/// Helper struct to convert references to timeseries. +/// +/// Keeps a reference to the original parameter name and the new timeseries reference. If the +/// timeseries refers to a table then the original parameter name is no longer required in the +/// final model, but is needed during conversion to ensure that the table is correctly referenced. +#[derive(Clone)] +pub struct ConvertedTimeseriesReference { + pub original_parameter_name: String, + pub ts_ref: TimeseriesReference, +} + +impl TryFromV1 for ConvertedTimeseriesReference { type Error = ConversionError; fn try_from_v1( @@ -328,6 +339,7 @@ impl TryFromV1 for TimeseriesReference { conversion_data: &mut ConversionData, ) -> Result { let meta: ParameterMeta = v1.meta.into_v2(parent_node, conversion_data); + let mut ts_name = meta.name.clone(); if let Some(url) = v1.url { // If there is a URL then this entry must be converted into a timeseries @@ -354,8 +366,10 @@ impl TryFromV1 for TimeseriesReference { if !conversion_data.timeseries.iter().any(|ts| ts.meta.name == meta.name) { conversion_data.timeseries.push(timeseries); } - } else if v1.table.is_some() { - // Nothing to do here; The table will be converted separately + } else if let Some(table) = v1.table { + // If this is a reference to a table then we need to point to the table by name, and + // ignore the original parameter's name entirely. + ts_name = table; } else { return Err(ConversionError::MissingAttribute { attrs: vec!["url".to_string(), "table".to_string()], @@ -371,11 +385,10 @@ impl TryFromV1 for TimeseriesReference { (None, None) => None, }; // The reference that is returned - let reference = TimeseriesReference { - name: meta.name, - columns, - }; - - Ok(reference) + let reference = TimeseriesReference { name: ts_name, columns }; + Ok(ConvertedTimeseriesReference { + original_parameter_name: meta.name, + ts_ref: reference, + }) } } diff --git a/pywr-schema/tests/test_models.rs b/pywr-schema/tests/test_models.rs index f75c80d3..22bc7d50 100644 --- a/pywr-schema/tests/test_models.rs +++ b/pywr-schema/tests/test_models.rs @@ -139,6 +139,5 @@ fn convert_model(v1_path: &Path, v2_path: &Path) { let v2_expected_str = fs::read_to_string(v2_path).unwrap(); let v2_expected: serde_json::Value = serde_json::from_str(&v2_expected_str).unwrap(); - assert_eq!(v2_converted, v2_expected); } diff --git a/pywr-schema/tests/v1/timeseries-converted.json b/pywr-schema/tests/v1/timeseries-converted.json index a7ef4b1b..af15f10d 100644 --- a/pywr-schema/tests/v1/timeseries-converted.json +++ b/pywr-schema/tests/v1/timeseries-converted.json @@ -43,6 +43,23 @@ "parameters": null, "cost": null }, + { + "type": "Input", + "max_flow": { + "type": "Timeseries", + "name": "inflow-tbl", + "columns": { + "type": "Column", + "name": "inflow1" + } + }, + "meta": { + "name": "input3" + }, + "min_flow": null, + "parameters": null, + "cost": null + }, { "type": "Link", "max_flow": null, @@ -82,6 +99,10 @@ "from_node": "input2", "to_node": "link1" }, + { + "from_node": "input3", + "to_node": "link1" + }, { "from_node": "link1", "to_node": "output1" diff --git a/pywr-schema/tests/v1/timeseries.json b/pywr-schema/tests/v1/timeseries.json index 9eda969d..a6f9b735 100644 --- a/pywr-schema/tests/v1/timeseries.json +++ b/pywr-schema/tests/v1/timeseries.json @@ -1,59 +1,89 @@ { - "metadata": { - "title": "Simple timeseries" + "metadata": { + "title": "Simple timeseries" + }, + "timestepper": { + "start": "2021-01-01", + "end": "2021-01-31", + "timestep": 1 + }, + "nodes": [ + { + "name": "input1", + "type": "Input", + "max_flow": "inflow" }, - "timestepper": { - "start": "2021-01-01", - "end": "2021-01-31", - "timestep": 1 + { + "name": "input2", + "type": "Input", + "max_flow": "factored_flow" }, - "nodes": [ - { - "name": "input1", - "type": "Input", - "max_flow": "inflow" - }, - { - "name": "input2", - "type": "Input", - "max_flow": "factored_flow" - }, - { - "name": "link1", - "type": "Link" - }, - { - "name": "output1", - "type": "Output", - "max_flow": "demand", - "cost": -10 - } + { + "name": "input3", + "type": "Input", + "max_flow": "inflow3" + }, + { + "name": "link1", + "type": "Link" + }, + { + "name": "output1", + "type": "Output", + "max_flow": "demand", + "cost": -10 + } + ], + "edges": [ + [ + "input1", + "link1" + ], + [ + "input2", + "link1" ], - "edges": [ - ["input1", "link1"], - ["input2", "link1"], - ["link1", "output1"] + [ + "input3", + "link1" ], - "parameters": { - "demand": { - "type": "constant", - "value": 100.0 - }, - "inflow": { - "type": "dataframe", - "url" : "inflow.csv", - "parse_dates": true, - "dayfirst": true, - "index_col": 0, - "column": "inflow1" - }, - "factored_flow": { - "type": "aggregated", - "agg_func":"product", - "parameters": [ - "inflow", - 0.5 - ] - } + [ + "link1", + "output1" + ] + ], + "parameters": { + "demand": { + "type": "constant", + "value": 100.0 + }, + "inflow": { + "type": "dataframe", + "url": "inflow.csv", + "parse_dates": true, + "dayfirst": true, + "index_col": 0, + "column": "inflow1" + }, + "inflow3": { + "type": "dataframe", + "table": "inflow-tbl", + "column": "inflow1" + }, + "factored_flow": { + "type": "aggregated", + "agg_func": "product", + "parameters": [ + "inflow", + 0.5 + ] + } + }, + "tables": { + "inflow-tbl": { + "url": "inflow.csv", + "parse_dates": true, + "dayfirst": true } + } }