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

Should we or shouldn't we save an inp file from SWMM? #46

Closed
kakila opened this issue Jun 21, 2017 · 54 comments
Closed

Should we or shouldn't we save an inp file from SWMM? #46

kakila opened this issue Jun 21, 2017 · 54 comments

Comments

@kakila
Copy link

kakila commented Jun 21, 2017

Hi,

I am trying to assimilate the intended workflow with the swwm python tools. Questions I could not answer by scanning the source tree and the docs there:

  1. How should tools/ by installed for the python search path?

  2. Are this tools within a module? e.g. can one do from .tools import swwm-reader?

  3. What python versions are supported? python 3?

I have developed a random network generator and I would be happy to make it compatible with your tools if I understand how to use them :D

Thanks

@bemcdonnell
Copy link
Member

@kakila, Have you checked out pyswmm? https://github.com/OpenWaterAnalytics/pyswmm

@bemcdonnell
Copy link
Member

@kakila, the python tools distributed with this repo are mainly for regression testing. Maybe I misunderstood what you're trying to do.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@bemcdonnell your pointer is the answer. Thanks, I somehow missed it.

During the project I will write some wrappers for the Octave programming language. Just to know, are you interested in such wrappers? (using OCT files).

@bemcdonnell
Copy link
Member

bemcdonnell commented Jun 21, 2017

@kakila, no problem. If you want to develop your octave wrapper "here" at OpenWaterAnalytics, we would be happy to facilitate.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@bemcdonnell great, thnkas. I will consder that.
My needs are the following:

  1. Write out INP files from Octave/Python data types

  2. Read out INP file into Octave/Python data types

  3. Modify INP Octave/Python representation

These represent wrappers to swmm_open and whatever function is used to save inp file (haven't identify it yet).

All the running part I would just use pySWMM because it seems quite complete. I would leave the part of interfacing swmm5 directly from octave (pySWMM style) as a GSoC project or as part of master thesis (we are getting a master student in August)... it might even be a wrapper of pySWMM if we get pytave running by then.

@bemcdonnell
Copy link
Member

@kakila - what if we work toward moving away from the traditional input files completely? Currently the API support load an existing model and allows you to manipulate that model within the SWMM data model. One of the projects we are going to be working on is the input API. This would allow you to initialize a model, then add H&H components as you feel through the API. This wont happen overnight but it might be worth considering so that we need not break apart and reconstruct input files.

PySWMM could be a nice dependency for any of your proposed wrappers. Another project might be worth considering is @aerispaha's swmmio package.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

  • Removing input files from the loop: how would you then share models? Would you share a python script that defines the model on the run? I think that it would be always wise to be able to produce that model as a file, although need not be the SWMM format. Question is whether it should be human readable...i am inclined to think it shouldn't but I would like to hear what you think. In any case people would need to load their old INP files tinto the tool that means we need to be INP-file aware, anyways. Did I understand you correctly?

  • I just skimmed the package swmmio, it seems it offers similar functionalities as pySWMM. Maybe it is wise that you join forces to make them pluggable and avoid over doing the work (seem both have representations of the INP file). However I do not see tools to write down new INP files (there is an almost empty inp class)

My idea of generating INP files is to be language independent. We develop the algos and then people can implement them in the language their prefer. Of course if you have an INP representation in your program, one could adapt the algos to that, is all equivalent. So it might be just a another task to have methods that take a particular INP representation as inputs, instead of a file.

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

@kakila we will be making bindings to Swmm and Swmmapi via swig moving forward, so many other languages can hook to swmm. Also generating an input file (which has been the traditional approach) is error prone and might change with subsequent versions. As @bemcdonnell points out

One of the projects we are going to be working on is the input API. This would allow you to initialize a model, then add H&H components as you feel through the API.

And then we can call the swmm routine that generates the file (that lives in the swmm code not on the bindings) and you an share that. The idea is to call this routine from the binding not to implement from scratch in some other language the process of converting a memory model to a .inp file.

Different languages follow different paradigms so each wrapper is free to create a layer on top of the C bindings to adjust the model to whatever make sense in that language.

Now regarding your needs

Write out INP files from Octave/Python data types

This is not how we plan to do this. Bindings will expose swmm internal netowrk model and routines to output the network in a inp file. We will probably add a layer on top of this with pyswmm to make the model pythonic

Read out INP file into Octave/Python data types

You can already load a model using pyswmm.

Modify INP Octave/Python representation

This is what we need to work on so we can do that directly in the C SWMM model and not on the bindings, that is the real way of making the whole process language independent via bindings.


Reimplementing in <insert language of choice here> the process of parsing an input file, storing a custom <insert language of choice here> network model and then regenerating a new input file via a custom module (re)implemented in <insert language of choice here> is not what we encourage to do, nor will do. Of course if you have pressing needs this is probably your only choice for a few months while the work is finished.

@bemcdonnell
Copy link
Member

PySWMM is only responsible for passing an Input file to the SWMM data model. From there, pyswmm uses the SWMM-toolkitAPI to manipulate that model. So PySWMM is working within the SWMM5 data model directly; not pre-generating an input file.

I am not suggesting we get rid of model input files completely - I'm saying that within SWMM we can expose a collection of functions that allow the user to load a model from anything

Just a concept at this point:

model = swmm_initProject();

swmm_addNode(model, 'A1', params);
swmm_addNode(model, 'A2', params);

swmm_addLink(model, "A1:A2", params);

swmm_addSubcatchment(model, "S1", "A1", params);

swmm_run(model);

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

On top of what @bemcdonnell exposes we would also have

...
swmm_export(model, 'somename.inp')  # or swmm_write or swmm_save

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@goanpeca I was never a friend of SWIG because development is really cumbersome. Also I had noticed unnecessary overheads due to the quality of the code generated. I assume these will be improved over time. This a developer decision, so it is alright.

I fully agree on having the model generated as a script in the language X instead of a file. I also think is a very good idea to centralize the INP output (btw, what's the swmm function?). I am not sure how easy is to make all this language "independent", and I am afraid this will end up being too abstract (again hinder development by the "non-experts" and the potential large community of users). I might be completely wrong.

Hence, when we are doing the development we can stick to your idea (and if you have by that time to your scripting definitions). I see however, that we still need a translation layer from XX to your scripting tools. Example: we exploit networkX set of algorithms for graph generation and analysis, hence we will need the mapping networkX --> your scripting tools.

@bemcdonnell
Copy link
Member

@kakila, I'm not suggesting an input file generator is a bad idea. What I am suggesting is that maybe the time to be spent on building an input file generator could be used for working on the input API in SWMM since this is the direction we are moving in.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@bemcdonnell roger that! I meant that if we can we will contribute to the input API (do you already have code? is this one?)

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

@goanpeca I was never a friend of SWIG because development is really cumbersome. Also I had noticed unnecessary overheads due to the quality of the code generated. I assume these will be improved over time. This a developer decision, so it is alright.

@kakila getting swig right the first time is a bit annoying and the automatic bindings that are generated are ugly, but who cares? it is an automatic tool, so we need to get it right once and then we have bindings available for a LOT of languages not just python. From here on each community/language will be in charge of taking their wrapper wherever they want to operate with the tools of that specific language. On this particular case, since we use and like python a lot we are in charge of making the python (pyswmm) bindings where it needs to go in a pythonic fashion and in a way it can operate with the python ecosystem and tools.

Hence, when we are doing the development we can stick to your idea (and if you have by that time to your scripting definitions). I see however, that we still need a translation layer from XX to your scripting tools. Example: we exploit networkX set of algorithms for graph generation and analysis, hence we will need the mapping networkX --> your scripting tools.

I don't really follow what you are trying to say here.

@bemcdonnell
Copy link
Member

bemcdonnell commented Jun 21, 2017

@kakila,

under the toolkitAPI branch is where the toolkitAPI is living for SWMM:

https://github.com/OpenWaterAnalytics/Stormwater-Management-Model/blob/toolkitapi/src/toolkitAPI.c

Our Friends over on the EPANET side (here at OWA) have put up some code on their net-builder (input API)

OpenWaterAnalytics/EPANET#88

@aerispaha
Copy link
Member

For what it's worth, over in the swmmio project I do currently have a way to write new INP files in the swmmio.utils.modify_model module. An example of this is provided in the README, where a section of the INP is adjusted via Pandas and reinserted into a copy of an existing INP.

@kakila, this has been very useful and may be helpful if you need this functionality immediately, but my opinion is that it's clunkier than what is planned for PySWMM.

I'm signed up to work on the RPT parser based on previous work in swmmio; maybe INP dealings in swmmio can also be used as a starting point/augmentation to the INP API work in PySWMM.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@goanpeca I was never a friend of SWIG because development is really cumbersome. Also I had noticed unnecessary overheads due to the quality of the code generated. I assume these will be improved over time. This a developer decision, so it is alright.
@kakila getting swig right the first time is a bit annoying and the automatic bindings that are generated are ugly, but who cares? it is an automatic tool, so we need to get it right once and then we have bindings available for a LOT of languages not just python.

@goanpeca You start caring when the performance of that ugly code gets in the way. Again, this is a preference, so go ahead.

@goanpeca Hence, when we are doing the development we can stick to your idea (and if you have by that time to your scripting definitions). I see however, that we still need a translation layer from XX to your scripting tools. Example: we exploit networkX set of algorithms for graph generation and analysis, hence we will need the mapping networkX --> your scripting tools.
@kakila I don't really follow what you are trying to say here.

@goanpeca You are defining an API for a swmm model. The representation is not uniquely defined (as you can see in the representations in swmio and pyswmm...and ours) and the reference abstraction is the one used for INP files. We used another representation (networkX) and we write INP files by mapping one representation to the other. If you have an API we will have to do the same, but instead of writing out a file, we now provide scripting tools that have your API as a dependency. Or do you see another way?

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@aerispaha @goanpeca looking again I think your API would benefit from networkX, unless you want to re-wrtie graph(even if tree) representations and algorithms (as @aerispaha seem to have done)

@bemcdonnell
Copy link
Member

@kakila,

here is what I am envisioning:

You have your model loaded in NetworkX. Next you use PySWMM as a hub to push your nodes and links directly into SWMM5's data model. This last step requires the Input API in SWMM

@bemcdonnell
Copy link
Member

The theme of PySWMM is to provide the pythonic interface. I think pyswmm should be a dependency for plotters, and other analysis features that people have in mind.

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

@goanpeca You start caring when the performance of that ugly code gets in the way. Again, this is a preference, so go ahead.

SWIG automatically generated bindings are faster than accessing the DLLs directly with ctypes as ctypes carries itself an overhead, in the Python case for pyswmm, can't speak about other languages but for python this is the way it is so:

You start caring when the performance of that ugly code gets in the way

If we ever get to that point (which I don't see how given the ctypes performance limitation), it is still a benefit if we can make bindings available to many more languages besides python.


@kakila, swmm has an internal representation of a model a network and its objects, we may (or may not) use networkx as the python higher level wrapper on top of the lower level representation that SWMM provides (will be exposed), when we get to that point we will evaluate that.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

As said,

  1. We will contribute to the toolkitAPI if we find something missing.

  2. We will provide python version of our generator that depends on pySWMM for:

    1. Reading INP files
    2. Inserting the generated network into the SWMM data model for simulation
    3. Generating an INP file

@bemcdonnell
Copy link
Member

@kakila, Excellent!

@michaeltryby
Copy link

The EPANET engine can create an input file from its internal data model. This makes working on an input API easier since it is an easy way to visually check the engines internal model state. I would recommend adding this functionality to SWMM as part of the input API.

@dickinsonre
Copy link

@michaeltryby - an excellent idea for #SWMM5 and #SWMM or models in general

@bemcdonnell
Copy link
Member

bemcdonnell commented Jun 21, 2017

@michaeltryby, SWMM generating an input file might be moving backwards. I don't think moving forward that we should be promoting this file format since it is not where the "puck is going to be." If someone wants to export an input file, they should use a wrapper to generate the new input file. This is just my opinion.

@michaeltryby
Copy link

michaeltryby commented Jun 21, 2017

@bemcdonnell I've written an input API for EPANET twice. Both times being able to generate an input file directly from the engine data model greatly simplified development.

If I were writing the input api for SWMM. An inputfile writer would be my first task. Great way to familiarize yourself with the data model as well.

@samhatchett
Copy link

Hi dudes. I would agree in theory with @bemcdonnell about not "polluting" the engine with file I/O. However, having a C component that handles the input file generation/parsing is also handy. For epanet, I've advocated just breaking it into two different components.

if you need knowledge of (or access to) the data model in order to (de)serialize, then the API isn't good enough 😉

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

@michaeltryby, SWMM generating an input file might be moving backwards. I

I disagree 👎 @bemcdonnell

I never view this repo as JUST the engine, we can split in folders if it helps to conceptualize the difference, but we need input and output tools.


The data model is accessed directly throughout the code. It really needs to be encapsulated and accessed through an API as part of this exercise.

Yes, we definitely need to move in that direction. Unless we can completely generalize the components/model concepts, we will always have some degree of coupling or we end up defining some internal specification for this... (maybe it is worth the trouble)

@bemcdonnell
Copy link
Member

This has been a great conversation today. If the general consensus is that an INP save function within SWMM would bring value to a workflow (even if we have complete API access to the data model) then those parties interested should put forward the code to do it! I'm still interested in a file format agnostic model... but if I have an input API, I should be able to accomplish what I need. :-)

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

I think we do need to have a human readable format for the persistence of a given model. The current format is probably not the best, but is what we have, but we should consider what type of format would be more generic and fit our needs. We can look at TOML, YAML, JSON etc... and not reinvent a human readable format, but use a well known standard and we then already have the parser/writer for this.

# This is a TOML document.

title = "My awesome SWMM model"
version = '0.1.0'

[author]
name = "Gonzalo Peña"
creation_date = 1979-05-27T07:32:00-08:00 # First class dates

[nodes]

  # Indentation (tabs and/or spaces) is allowed but not required
  [nodes.n1]
  name = "some node name"
  x = '...'
  y = '...'
  z = '...'

  [nodes.n2]
  name = "some other node name"
  x = '...'
  y = '...'
  z = '...'

  [nodes.n3]
  name = "some other other node name"
  x = '...'
  y = '...'
  z = '...'

[links]

  # Indentation (tabs and/or spaces) is allowed but not required
  [links.l1]
  name = "some link name"
  from = 'n1'
  to = 'n2'

  [links.l2]
  name = "some other name"
  from = 'n2'
  to = 'n3'

@bemcdonnell
Copy link
Member

@goanpeca, I agree that human readable is important. Another thing I am interested in is a format that will be clean to pass through version control (SVN/GIT/et al.). Traditional diff tools do not work well with these text based model files since any space or change in the order screws everything up.

If you're trying to diff a model saved by SWMM-UI to the same model that was adjusted and saved by a commercial software package - normally you will be left with a heartache.

@goanpeca
Copy link

I agree that human readable is important. Another thing I am interested in is a format that will be clean to pass through version control (SVN/GIT/et al.). Traditional diff tools do not work well with these text based model files since any space or change in the order screws everything up.

The best way to achieve that is by having ordering/sorting of keys and using line per line formats...

@bemcdonnell
Copy link
Member

@goanpeca, I'm not sure that changing the format will really be an option in the near future.

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

I'm not sure that changing the format will really be an option in the near future.

Probably but we need to think about that for the not near future, a custom format will always force tight coupling between code and model

@aerispaha
Copy link
Member

@bemcdonnell, I agree with the importance of being able to use Git et el. on INP files. I deal with this in swmmio by comparing models element-by-element so that sorting and comments do not impact the diff. This fails, however, on some of the unusual sections in the INP such as [CURVES].

As an aside: in a very large model (9,000+ links), I've noticed that the way elements are sorted in the INP impacts some of the summary results, very slightly.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@bemcdonnell the export to INP file should be already part of the swmm code. I reiterate my question to anybody who might have an answer available: what is the function called when the user press 'Save' in the swmm GUI? So this output format should be at least available.

Regarding comments of output formats, I guess you start with one format that is useful for your purposes, currently for us is INP because hydrologist are used to it. When better formats for other situations are available (e.g. JSON, HDF5 or wathever) the interested party should come up with the formater (lets agree that writing an output formater is tedious but quite a simple task)

@goanpeca
Copy link

goanpeca commented Jun 21, 2017

Regarding comments of output formats, I guess you start with one format that is useful for your purposes, currently for us is INP because hydrologist are used to it. When better formats for other situations are available (e.g. JSON, HDF5 or wathever) the interested party should come up with the formater (lets agree that writing an output formater is tedious but quite a simple task)

The issue is more of using standards vs not using them. What is the INP file standard (I don't know)?

HDF5/NETCDF are actually pretty good formats for storing results. Don't know what SWMM uses for output files either

@bemcdonnell
Copy link
Member

@kakila, the UI has it's own data model and when you hit the "save" button, it exports a new file from its data model - it does not share a data model with SWMM.

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@goanpeca what do you refer to a standard? are you looking for a Committee that meets to define it or what do you mean? A format definition works as a standard and INP definitely has a definition (otherwise we would not behaving this discussion). Do you want ti described as a Grammar?

@bemcdonnell oh, bumps! Anyways where is that file? We could reuse if the code is well structured, if not we could just put down to C/C++t our current writer.

@bemcdonnell
Copy link
Member

@kakila, the UI is written in Pascal, not C. :-)

@bemcdonnell
Copy link
Member

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@bemcdonnell oh well, so much for the hope of reducing the work. Ok, we will see to provide a C/C++ INP writer, guess that is task 0.

At first glance, using std:: containers for the different sections of the files seems reasonable. Is everybody interested ok with that?

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@bemcdonnell not completely bad news! The file implementing the INP format is Uexport.pas in the GUI folder. It is very good code, very modular! Porting to C/C++ shouldn't be that hard (still tedious). Thanks

@bemcdonnell
Copy link
Member

bemcdonnell commented Jun 21, 2017

@kakila, lets open a new issue for this and begin the talk there. #47

@lmontest
Copy link

lmontest commented Jun 21, 2017 via email

@kakila
Copy link
Author

kakila commented Jun 21, 2017

@lmontest I do not completely grasp your stand. Do you want to be able to write an INP file from the api?(altough techically wont belong to the api but to the io tools) You being kind of saying no, but then you say just that. Sorry for not understanding

@samhatchett
Copy link

I think @lmontest is suggesting, as others have, that the file I/O should be separate from the engine library - and that the file writer should use the engine API to interrogate the project for data and write it out. sound right?

@bemcdonnell
Copy link
Member

@samhatchett, you are correct

@lmontest
Copy link

lmontest commented Jun 22, 2017 via email

@bemcdonnell bemcdonnell reopened this Jun 22, 2017
@bemcdonnell bemcdonnell changed the title [Help] intended workflow for python tools Should we or shouldn't we save an input from SWMM? Jun 22, 2017
@bemcdonnell bemcdonnell changed the title Should we or shouldn't we save an input from SWMM? Should we or shouldn't we save an inp file from SWMM? Jun 22, 2017
@goanpeca
Copy link

goanpeca commented Jun 22, 2017

Yes, I think that inp file manipulation should be done outside the engine and as part of an io library.

It should not be part of the engine... but it should/could be part of this library and this repository and it must be C or C++, so that there is ONE default implementation that can be used by other wrappers, of which python is just one and the one we like most.

There needs to be a standard way to serialize a model into a human readable file. It could be the current format or a new one and other output formats (xml json...) available, but making this in python (and I am a huge python fan!) will limit the community that can access this a lot.


I suggested on another issue to organize the repository to reflect this

/src/engine -> *.c
/src/io -> *.c
/src/tools -> *.c
/headers -> *.h

It can be eventually a separate library on a separate repo, when it is clearly defined and it is designed to be used for epanet and swmm, before that it will be a big pain to separate the code to another repository and a separate library. Synchronization is already a mess between this repo and PySwmm.


Just look at an example from another domain. SASS (a CSS preprocessor) was originally written in ruby and after demand for other languages, it was rewritten in C++ and now it has many different bindings on different languages. https://github.com/sass/libsass

@bemcdonnell
Copy link
Member

#47 for implementation discussion

michaeltryby added a commit to SWMM-Project/swmm-solver that referenced this issue Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants