search

pyswmm / Stormwater-Management-Model

PySWMM Stormwater Management Model repository
Other
99 stars 77 forks source link

Modified API for initial conditions #392

Closed bemcdonnell closed 1 year ago

bemcdonnell commented 1 year ago

First part in addressing pyswmm-445

@karosc, i learned a new command to type before making a commit to swmm... if you have a million white space changes, this command saves you git diff -w | git apply --cached --ignore-whitespace

This will solve the initial conditions problem for network levels. The "settings" are solved just by organizing the sim.start()

karosc commented 1 year ago

@bemcdonnell, Node.initDepth is only used for setting the initial node depth in node_initState(), which is called in project_init(), which is called in swmm_start().

From my point of view, allowing the API to set the initial depth after the simulation start would have no effect on the initial conditions since oldDepth and newDepth will have already been set https://github.com/pyswmm/Stormwater-Management-Model/blob/9bd04f719ea6580d73f8ae117108068aef8d68f5/src/solver/node.c#L245

Looking at the pyswmm api, I'd be in favor of deprecating the initial_conditions property in favor of an additional callback: Simulation.after_start. This would give users a clear path forward when they encounter errors when trying to set initial conditions at the wrong point in the simulation flow. Some props can only be set prior to simulation start (e.g. initDepth), while some must be set after start (e.g. target_setting). Ideally pyswmm throws a reasonably descriptive error that can point users to the simulation state (i.e. opened, started, closed) in which setters are valid.

bemcdonnell commented 1 year ago

@karosc, I can't help but agree with your idea. the "after_start" in pyswmm is a great idea. I think when i wrote those callback hooks, "before_step" was to accomplish the same thing. And you're correct, if adding an "after_start" will allow the target_settings to be made. I will close this PR and just make the changes in PySWMM. Thanks for the review!