Add Turbine and HydropowerDam nodes

[jetuk]

One of the purposes of this version is to allow the design of more useful high level nodes. To that end, and related to #105 , are the implementation of Turbine and HydropowerDam nodes.

Here are some pseudo schemas before writing the actual implementation.

struct TurbineNode {
    // This is probably a copy of what's in `HydropowerTargetParameter`?
    // Perhaps `target` is an `Option<_>` for the turbine to operate on whatever flow goes through (rather than targeting a power output),

    // Attributes: InFlow, OutFlow, Power, Head?, 
}

struct HydropowerDam {
    pub meta: NodeMeta,
    pub max_volume: Option<DynamicFloatValue>,
    pub min_volume: Option<DynamicFloatValue>,
    pub cost: Option<DynamicFloatValue>,
    pub initial_volume: StorageInitialVolume,

    // Mostly attributes from `HydropowerTargetParameter`

    // Replace `water_elevation` with bathymetry.
    // This could be pairs of points (volume, level) for interpolation, or
    // Separate `volume` and  `elevation` attributes (but the user would have ensure the align and are the same length).
    pub bathymetry: Vec<(_, _)>

    // The number of turbines
    pub units: usize

    // Attributes: Volume, ProportionalVolume, Level, Power, Head?, 
}

[s-simoncelli]

I cna start looking at implementing the TurbineNode. I've got a few questions about its implementation:

• Is this going to be a Link node with some additional attributes? (like power which can be calculated from current flow and other struct fields)
• If the (power) target is set, is this supposed to set the node max_flow?

[jetuk]

Yes, the schema node will create a Link in the "core" model. I think if the target is set then, yes, it will use the parameter you have already written to calculate a target flow. It could have some flag to set the target as a min flow too? That's a recipe for not having enough flow though.

We should also add NodeAttribute::Power which will be supported in TurbineNode::create_metric to return the calculated power via a new DerivedMetric::Power. This last bit might be more difficult than I think.

I think this where we'll benefit from having the power functions split out as simple functions. They will be used by HydropowerTargetParameter and DerivedMetric::Power.

[s-simoncelli]

I am still trying to get my head around metrics and how they are handled. What if I create something like this:

impl TurbineNode {
    ...
    pub fn set_constraints(
        &self,
        network: &mut pywr_core::network::Network,
        schema: &crate::model::PywrNetwork,
        domain: &ModelDomain,
        tables: &LoadedTableCollection,
        data_path: Option<&Path>,
        inter_network_transfers: &[PywrMultiNetworkTransfer],
    ) -> Result<(), SchemaError> {
        if let Some(cost) = &self.cost {
            let value = cost.load(network, schema, domain, tables, data_path, inter_network_transfers)?;
            network.set_node_cost(self.meta.name.as_str(), None, value.into())?;
        }

        if let Some(target) = &self.target {
            let name = format!("{}-power", self.meta.name.as_str());
            let p = pywr_core::parameters::HydropowerTargetParameter::new(
                &name,
                target,
                &self.water_elevation,
                &self.turbine_elevation,
                &self.min_head,
                &self.max_flow,
                &self.min_flow,
                &self.efficiency,
                &self.water_density,
                &self.flow_unit_conversion,
                &self.energy_unit_conversion,
            );
            let power_idx = network.add_parameter(Box::new(p))?;
            let metric = Metric::ParameterValue(power_idx);
            network.set_node_max_flow(self.meta.name.as_str(), None, metric.clone().into())?;
        }

        Ok(())
    }
}

Wouldn't it make more sense to have a NodeFlowFromPower as derived metric?

[jetuk]

Yes, this looks good. This opens an issue about parameter names. I would like to eventually remove the need for making up names like this. @Batch21 has come across a similar issue in #99 . I suggest we leave it as is now, but put a TODO on that line let name = ... to say this needs address with name-spacing.

Re the derived metric. Won't it be PowerFromNodeFlow? I.e. after solve we'll know the flow that was allocated and we might want to compute the actual power (it could be different to what was specified in the parameter above). The issue being you'll need all of the data to do that calculation in the derived metric. It might make sense to create a Turbine struct to contain all that data to be easily passed around (I think clippy will also complain your current HydropowerTargetParameter::new function has too many arguments).