-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement a system of constant parameters. #62
Comments
Current Parameter calculation method signature: pub trait Parameter<T>: Send + Sync {
fn compute(
&self,
timestep: &Timestep,
scenario_index: &ScenarioIndex,
model: &Network,
state: &State,
internal_state: &mut Option<Box<dyn ParameterState>>,
) -> Result<T, PywrError>;
} Alternative signatures could depend on less data: /// Parameter who's value does not change but could be dependent on another `ConstantParameter`
/// This can be evaluated once at the start of a model run.
pub trait ConstantParameter<T>: Send + Sync {
/// The output of this would be stored in `ConstantParameterValues`
fn compute(
&self,
scenario_index: &ScenarioIndex,
state: &ConstantParameterValues
) -> Result<T, PywrError>;
}
/// Parameter who's value can change in time (could be called something else)
/// This can be first at the start of a time-step
pub trait SimpleParameter<T>: Send + Sync {
/// The output of this would be stored in `SimpleParameterValues`
fn compute(
&self,
timestep: &Timestep,
state: &SimpleParameterValues
) -> Result<T, PywrError>;
} |
The idea here is that the algorithm would change to something like:
The middle part is more like Pywr v1.x with nodes updated before parameters. The current implementation here tries to mix them together to have dependency tree resolve in the correct order. Which is why you have to load max volume before creating the storage node (see also #192). In this method that would no longer be needed as there would be a restriction on max volume to only allow simple or constant parameters. This feels like it models the invariant (i.e. max volume can't be dependent on the node) better with the type system. |
Done in #194 |
The current parameter system supports values that change during a simulation (i.e. over the time and scenario domains). Several parameter types do not vary in either or both of these domains (e.g.
ConstantParameter
). There's an efficiency to be gained from only "calculating" such parameters once at the beginning of a simulation.However, it might also be useful to have constants derived from other constants. For example, where a
ConstantParameter
is used to optimise (i.e. by running the model with many different values) the size of a reservoir an associated cost function might depend on that constant value. It would need to be re-calculated if the constant were to change before each simulation.This implementation could follow the same pattern as the regular parameter implementation. However, it would only depend on a subset of the state - the constant values. This subset would then be immutable for the remainder of the simulation, but could be used by regular parameters as required.
The text was updated successfully, but these errors were encountered: