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

Concept implementation of Network.run_interactive() method #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jbaayen
Copy link
Contributor

@jbaayen jbaayen commented Mar 12, 2018

We are dealing with an application where we have some of our controllers implemented in Python. These need to read the current system state at every time step, compute control output, and feed the control output back into EPANET.

In this concept implementation, control is passed back to the caller subsequent to every time step using yield().

I don't like the idea of having many flavours of run() methods and would therefore like to use this pull request to open the discussion as to the design of a generic runner method, able to satisfy all relevant use cases.

to dynamically inject input data on every time step.
@AbelHeinsbroek
Copy link
Member

I see the added value of an interactive runloop using yield(). I also agree that too many run flavours would add unnecessary complexity.

The solution I propose is refactoring the module to have one single run method, that takes all run-relevant parameters:

interactive: Boolean        # if set to True, use yield()
duration: Integer              # if set to 0, calculate steady state
hydraulic_step: Integer    
report_step: Integer
...etc..

We can still keep the existing solve steady-state function as a convienience method, calling the main run method with certain set parameters.

@jbaayen
Copy link
Contributor Author

jbaayen commented Mar 23, 2018

Sounds good to me. Will refactor accordingly.

generator, depending on the options specified.
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

Successfully merging this pull request may close these issues.

2 participants