fix: Try to convert RiverSplitWithGauge factors correctly.

pywr-schema/src/error.rs

@@ -126,4 +126,6 @@ pub enum ConversionError {
     TableRef { attr: String, name: String, error: String },
     #[error("Unrecognised type: {ty}")]
     UnrecognisedType { ty: String },
+    #[error("Non-constant value cannot be converted automatically.")]
+    NonConstantValue {},
 }

pywr-schema/src/nodes/river_split_with_gauge.rs

@@ -7,21 +7,31 @@ use crate::model::LoadArgs;
 use crate::nodes::{NodeAttribute, NodeMeta};
 use crate::parameters::Parameter;
 use crate::v1::{try_convert_node_attr, ConversionData, TryFromV1};
+use crate::{ConversionError, TryIntoV2};
 #[cfg(feature = "core")]
 use pywr_core::{aggregated_node::Relationship, metric::MetricF64, node::NodeIndex};
 use pywr_schema_macros::PywrVisitAll;
 use pywr_v1_schema::nodes::RiverSplitWithGaugeNode as RiverSplitWithGaugeNodeV1;
+use pywr_v1_schema::parameters::ParameterValues;
 use schemars::JsonSchema;
 
 #[derive(serde::Deserialize, serde::Serialize, Clone, Debug, JsonSchema, PywrVisitAll)]
 pub struct RiverSplit {
+    /// Proportion of flow not going via the mrf route.
     pub factor: Metric,
+    /// Name of the slot when connecting to this split.
     pub slot_name: String,
 }
 
 #[doc = svgbobdoc::transform!(
-/// This is used to represent a proportional split above a minimum residual flow (MRF) at a gauging station.
+/// A node used to represent a proportional split above a minimum residual flow (MRF) at a gauging station.
 ///
+/// The maximum flow along each split is controlled by a factor. Internally an aggregated node
+/// is created to enforce proportional flows along the splits and bypass.
+///
+/// **Note**: The behaviour of the factors is different to this in the equivalent Pywr v1.x node.
+/// Here the split factors are defined as a proportion of the flow not going via the mrf route.
+/// Whereas in Pywr v1.x the factors are defined as ratios.
 ///
 /// ```svgbob
 ///           <node>.mrf
@@ -117,7 +127,7 @@ impl RiverSplitWithGaugeNode {
     pub fn node_indices_for_constraints(
         &self,
         network: &pywr_core::network::Network,
-    ) -> Result<Vec<pywr_core::node::NodeIndex>, SchemaError> {
+    ) -> Result<Vec<NodeIndex>, SchemaError> {
         // This gets the indices of all the link nodes
         // There's currently no way to isolate the flows to the individual splits
         // Therefore, the only metrics are gross inflow and outflow
@@ -245,15 +255,17 @@ impl TryFromV1<RiverSplitWithGaugeNodeV1> for RiverSplitWithGaugeNode {
         let mrf = try_convert_node_attr(&meta.name, "mrf", v1.mrf, parent_node, conversion_data)?;
         let mrf_cost = try_convert_node_attr(&meta.name, "mrf_cost", v1.mrf_cost, parent_node, conversion_data)?;
 
-        let splits = v1
-            .factors
+        let factors = convert_factors(v1.factors, parent_node, conversion_data).map_err(|error| {
+            ComponentConversionError::Node {
+                attr: "factors".to_string(),
+                name: meta.name.to_string(),
+                error,
+            }
+        })?;
+        let splits = factors
             .into_iter()
-            .skip(1)
             .zip(v1.slot_names.into_iter().skip(1))
-            .map(|(f, slot_name)| {
-                let factor = try_convert_node_attr(&meta.name, "factors", f, parent_node, conversion_data)?;
-                Ok(RiverSplit { factor, slot_name })
-            })
+            .map(|(factor, slot_name)| Ok(RiverSplit { factor, slot_name }))
             .collect::<Result<Vec<_>, Self::Error>>()?;
 
         let n = Self {
@@ -266,3 +278,40 @@ impl TryFromV1<RiverSplitWithGaugeNodeV1> for RiverSplitWithGaugeNode {
         Ok(n)
     }
 }
+
+/// Try to convert ratio factors to proprtional factors.
+fn convert_factors(
+    factors: ParameterValues,
+    parent_node: Option<&str>,
+    conversion_data: &mut ConversionData,
+) -> Result<Vec<Metric>, ConversionError> {
+    let mut iter = factors.into_iter();
+    if let Some(first_factor) = iter.next() {
+        if let Metric::Constant { value } = first_factor.try_into_v2(parent_node, conversion_data)? {
+            // First Metric is a constant; we can proceed with the conversion
+
+            let split_factors = iter
+                .map(|f| {
+                    if let Metric::Constant { value } = f.try_into_v2(parent_node, conversion_data)? {
+                        Ok(value)
+                    } else {
+                        Err(ConversionError::NonConstantValue {})
+                    }
+                })
+                .collect::<Result<Vec<_>, _>>()?;
+
+            // Convert the factors to proportional factors
+            let sum: f64 = split_factors.iter().sum::<f64>() + value;
+            Ok(split_factors
+                .into_iter()
+                .map(|f| Metric::Constant { value: f / sum })
+                .collect())
+        } else {
+            // Non-constant metric can not be easily converted to proportional factors
+            Err(ConversionError::NonConstantValue {})
+        }
+    } else {
+        // No factors
+        Ok(vec![])
+    }
+}

pywr-schema/tests/test_models.rs

@@ -125,6 +125,7 @@ macro_rules! convert_tests {
 convert_tests! {
     test_convert_timeseries: ("v1/timeseries.json", "v1/timeseries-converted.json"),
     test_convert_inline_parameter: ("v1/inline-parameter.json", "v1/inline-parameter-converted.json"),
+    test_convert_river_split_with_gauge1: ("v1/river_split_with_gauge1.json", "v1/river_split_with_gauge1-converted.json"),
 }
 
 fn convert_model(v1_path: &Path, v2_path: &Path) {

pywr-schema/tests/v1/river_split_with_gauge1-converted.json

@@ -0,0 +1,123 @@
+{
+  "metadata": {
+    "title": "RiverSplitWithGauge",
+    "description": "Example of an abstraction with an MRF of form y=mx+c",
+    "minimum_version": "0.1"
+  },
+  "timestepper": {
+    "start": "2015-01-01",
+    "end": "2015-12-31",
+    "timestep": 1
+  },
+  "scenarios": null,
+  "network": {
+    "nodes": [
+      {
+        "meta": {
+          "name": "Catchment"
+        },
+        "type": "Catchment",
+        "cost": null,
+        "flow": {
+          "type": "Constant",
+          "value": 100.0
+        },
+        "parameters": null
+      },
+      {
+        "meta": {
+          "name": "Gauge"
+        },
+        "type": "RiverSplitWithGauge",
+        "mrf": {
+          "type": "Parameter",
+          "name": "Gauge-p0",
+          "key": null
+        },
+        "mrf_cost": {
+          "type": "Constant",
+          "value": -1000.0
+        },
+        "parameters": null,
+        "splits": [
+          {
+            "factor": {
+              "type": "Constant",
+              "value": 0.25
+            },
+            "slot_name": "abstraction"
+          }
+        ]
+      },
+      {
+        "meta": {
+          "name": "Estuary"
+        },
+        "type": "Output",
+        "cost": null,
+        "max_flow": null,
+        "min_flow": null,
+        "parameters": null
+      },
+      {
+        "meta": {
+          "name": "Demand"
+        },
+        "type": "Output",
+        "max_flow": {
+          "type": "Constant",
+          "value": 50.0
+        },
+        "cost": {
+          "type": "Constant",
+          "value": -10.0
+        },
+        "min_flow": null,
+        "parameters": null
+      }
+    ],
+    "edges": [
+      {
+        "from_node": "Catchment",
+        "to_node": "Gauge"
+      },
+      {
+        "from_node": "Gauge",
+        "from_slot": "river",
+        "to_node": "Estuary"
+      },
+      {
+        "from_node": "Gauge",
+        "from_slot": "abstraction",
+        "to_node": "Demand"
+      }
+    ],
+    "metric_sets": null,
+    "parameters": [
+      {
+        "meta": {
+          "name": "Gauge-p0"
+        },
+        "type": "MonthlyProfile",
+        "interp_day": null,
+        "values": [
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0,
+          40.0
+        ]
+      }
+    ],
+    "outputs": null,
+    "tables": null,
+    "timeseries": null
+  }
+}

pywr-schema/tests/v1/river_split_with_gauge1.json

@@ -0,0 +1,45 @@
+{
+    "metadata": {
+        "title": "RiverSplitWithGauge",
+        "description": "Example of an abstraction with an MRF of form y=mx+c",
+        "minimum_version": "0.1"
+    },
+    "timestepper": {
+        "start": "2015-01-01",
+        "end": "2015-12-31",
+        "timestep": 1
+    },
+    "nodes": [
+        {
+            "name": "Catchment",
+            "type": "catchment",
+            "flow": 100
+        },
+        {
+            "name": "Gauge",
+            "type": "RiverSplitWithGauge",
+            "mrf": {
+                "type": "monthlyprofile",
+                "values": [40.0, 40.0, 40.0, 40.0, 40.0, 40.0, 40.0, 40.0, 40.0, 40.0, 40.0, 40.0]
+            },
+            "mrf_cost": -1000,
+            "factors": [3, 1],
+            "slot_names": ["river", "abstraction"]
+        },
+        {
+            "name": "Estuary",
+            "type": "output"
+        },
+        {
+            "name": "Demand",
+            "type": "Output",
+            "max_flow": 50,
+            "cost": -10
+        }
+    ],
+    "edges": [
+        ["Catchment", "Gauge"],
+        ["Gauge", "Estuary", "river", null],
+        ["Gauge", "Demand", "abstraction", null]
+    ]
+}