Skip to content
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

use appropriate demand values for single-time water model #95

Closed
jjstickel opened this issue Jul 8, 2020 · 19 comments
Closed

use appropriate demand values for single-time water model #95

jjstickel opened this issue Jul 8, 2020 · 19 comments

Comments

@jjstickel
Copy link
Collaborator

When instantiating a single-time water model, the demands for junctions are currently set to the EPANET "base demand" values. However, the actual demand should come from multiplying base demand by the pattern. Since this is already computed as a time-series, the time-zero value can be used for demand.

I propose to change the existing demand filed be relabeled as base_demand. Then a new demand field can be created that uses the time-series value.

@jjstickel
Copy link
Collaborator Author

BTW, reservoir head can also be assigned a pattern (although it is less used than for junction demand). So it it should have the same fix.

@ccoffrin
Copy link
Member

ccoffrin commented Jul 8, 2020

If I follow the issue, I don't think we need a new field called base_demand. After loading an EPANET file, if would like the base demand, look in the network data. If you would like one of the demand profiles from the time-series load the time-zero value into the network. Or did I miss something?

@jjstickel
Copy link
Collaborator Author

There are a couple ways to store the information, and I am not sure what is the most logical. EPANET inp files provide base demand and pattern. Right now, these are stored as fields for each junction, and the time series is stored separately. When instantiating the model, I suggest using the time series value(s). If this is done, then I agree that base demand and patterns can be discarded. However, there may be some value in keeping them around?

It is important to note that base demand is not some nominal or "average" demand. It can be an order of magnitude different because the pattern multipliers can be very large or very small. So using base demand as "demand" in a model is not very useful.

@ccoffrin
Copy link
Member

ccoffrin commented Jul 8, 2020

As I understand it there is a one-to-one relationship between WaterModels and EPANET. base_demand goes into the network, patterns go into the time_series block.

As I understand it patterns are optional, if this is not present then we have to interpret base_demand as demand. So it seems consistent to interpret that way in the presence of patterns as well.

@ccoffrin
Copy link
Member

ccoffrin commented Jul 8, 2020

@tasseff what are your thoughts?

@jjstickel
Copy link
Collaborator Author

jjstickel commented Jul 8, 2020

As I understand it there is a one-to-one relationship between WaterModels and EPANET. base_demand goes into the network, patterns go into the time_series block.

Not quite. The time_series is base_demand times pattern. For the parsed data object, all three are currently stored.

As I understand it patterns are optional, if this is not present then we have to interpret base_demand as demand. So it seems consistent to interpret that way in the presence of patterns as well.

You are partially right: if a pattern does not exist (or is not assigned) for a junction, then demand = base_demand and is constant with time. However, as I mentioned in my last comment, if pattern exists, base_demand could be way off from the actual demand and is not meaningful by itself. Something from time_series should be used instead.

@tasseff
Copy link
Member

tasseff commented Jul 8, 2020

I think in general, if a time_series block is found in the (non-multinetwork) data but a single time period problem specification (e.g., build_wf) is invoked, a warning should appear to the user. However, transforming all fields that carry time_series information upon invocation of a single-period problem might be overkill. In general, the user should be expected to provide data that fits the type of problem being built.

@jjstickel
Copy link
Collaborator Author

jjstickel commented Jul 8, 2020

Expecting the user to provide a single-time inp file seems a bit much. You don't have to transform all "fields that carry time_series information", just pick a single time value. Again, I suggest taking time zero. A warning to the user would then be appropriate.

@ccoffrin
Copy link
Member

ccoffrin commented Jul 8, 2020

I think we may be having some confusion so I recomend we discuss this on the next team meeting, so we can discuss in detail and come to a consensus.

@tasseff
Copy link
Member

tasseff commented Jul 8, 2020

Looking at the I/O methods, I think the solution is easier than I thought. I will commit a fix shortly.

tasseff added a commit that referenced this issue Jul 8, 2020
@tasseff
Copy link
Member

tasseff commented Jul 8, 2020

See changes to io/epanet.jl in a4ccf3e. This changes (single-period) fields using time series data as per the recommendation above (i.e., default to values from the first time step).

I personally don't feel storing base_demand or the like is necessary.

@ccoffrin
Copy link
Member

I propose the following semantics,

  • The "base demands" specified in the EPANET files are loaded in the data using the keyword demand
  • The time series block includes values that are EPANET's "base demand" * "pattern scalar".
  • The parsing argument "import_all = true" can be used to extract extra information from EPANET that is not currently used or supported by WaterModels

A user who would like to analyze the single time point problem that is specified by EPANET's "base_demand" runs on the loaded data, as is. A user who would like to analyze the single time point problem that is specified by the pattern curves first should load a timepoint from the time series block, then run the analysis. A user who would like to analyze the multi-time point problem should turn the time series into a multi-network (as I recall some helper functions already exist for this case), then run the analysis.

These conventions are consistent with how this works in other infrastructure systems. I am not seeing a reason to deviate from these conventions at this point, but we can discuss further at the next meeting if needed.

@jjstickel
Copy link
Collaborator Author

jjstickel commented Jul 13, 2020

A user who would like to analyze the single time point problem that is specified by EPANET's "base_demand" runs on the loaded data, as is.

The base_demand (when there is a pattern) is not intended to be a realistic value for demand at a junction, so I think this use case should be avoided or at least warned against. This is also why I suggest that base demand should be labelled base_demand in the dicts.

A user who would like to analyze the single time point problem that is specified by the pattern curves first should load a timepoint from the time series block, then run the analysis.

@tasseff, Does user-friendly functionality exist to do this?

A user who would like to analyze the multi-time point problem should turn the time series into a multi-network (as I recall some helper functions already exist for this case), then run the analysis.

This part is fine and already implemented (as far as I understand).

@tasseff
Copy link
Member

tasseff commented Jul 13, 2020

@jjstickel, something like what Carleton is suggesting could be done via

data["junction"]["2"]["demand"] = data["time_series"]["junction"]["2"]["demand"][3]

which would replace the demand entry with the third demand from the time series.

To your original point that base_demand is not intended to be a realistic value for demand at a junction, digging into this a bit more, I've found that in most example networks, patterns for demand are mostly centered around scalars of one. (Here is a good set of example networks.) For this reason, I'm not sure that a base_demand entry is truly needed. Also, given this conventional centering around one, I'd like to revisit my previous commit and not always use the first time point's demand, as it feels arbitrary.

Finally, differentiating between base_demand (when a junction has a pattern) and demand (when it doesn't) seems unnecessarily complex. If we document what demand is according to our current conventions, there should be no problem.

@jjstickel
Copy link
Collaborator Author

I've found that in most example networks, patterns for demand are mostly centered around scalars of one.

I found at least one example where this is not the case, Net3.inp. This network is used in some specific examples in the repo that you provide a link to, msx-examples/Net3-Bio/ and msx-examples/Net3-NH2Cl. For some of the junctions with demands, the pattern multipliers are O(1000), to be multiplied by a base demand of 1. This is what I have had in mind during our discussions. Sorry to not have provided the example earlier.

It seems that you want to put the onus on the user to check for the reasonableness of the network definition. I can come to terms with that, but I feel like it would be fairly simple to make some minor modifications to help users catch these problems and/or fix it for them.

@ccoffrin
Copy link
Member

A user who would like to analyze the single time point problem that is specified by the pattern curves first should load a timepoint from the time series block, then run the analysis.

The function, InfrastructureModels.load_timepoint! should accomplish this for a user.

It seems that you want to put the onus on the user to check for the reasonableness of the network definition.

You may be right here. Generally I would strive for intuitive interfaces with very simple software semantics. If we cannot have both at the same time, I would usually err on the side of simple software semantics and expect higher levels of comprehension from the users.

From my view, the bigger challenge we are trying to resolve here is how to best have consistent conventions across Power, Water and Natural Gas infrastructures. These other two infrastructures follow the conventions I have outlined and users seem to be content with these semantics, so I think it can work here as well.

@jjstickel
Copy link
Collaborator Author

OK. I will defer to your motivation for consistency. In that case, the changes in a4ccf3e to set the demand to the first time-series value should be undone.

@jjstickel
Copy link
Collaborator Author

jjstickel commented Jul 13, 2020

I also agree that running InfrastructureModels.load_timepoint!(data, [time_index]) before instantiating a single-step model is a simple way out of this conundrum. Maybe open a separate ticket to document this somewhere?

@tasseff
Copy link
Member

tasseff commented Jul 14, 2020

It sounds like we have come to a consensus that the previous conventions will stand. I will document use of load_timepoint! in #96

@tasseff tasseff closed this as completed Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants