pyswmm / Stormwater-Management-Model

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

Add coordinate support to SWMM data model and expose API #285

Open mdbartos opened 6 years ago

mdbartos commented 6 years ago

Hey @bemcdonnell, just wanted to check and see if there is any desire for integrated GIS-like functionality in pyswmm.

Some functionality that could be convenient to have:

Doing these efficiently would probably require introducing some optional dependencies.

Let me know if you think these functions would be appropriate for pyswmm. If so, let me know what functionality you would like to see.

Thx, MDB

lrntct commented 6 years ago

This might be achieved by having a networkx (or similar) representation. IMHO, it could be interesting.

mdbartos commented 6 years ago

I have an implementation of upstream area delineation for DEMs here: https://github.com/mdbartos/pysheds/blob/master/pysheds/grid.py#L634

It ended up being a lot faster than explicitly constructing a graph. The only dependency is numpy (basically just used for fancy indexing). You could do it without numpy but it would probably be quite a bit slower.

bemcdonnell commented 6 years ago

@mdbartos, I wonder if it would make sense to have this inside SWMM itself. What do you think?

mdbartos commented 6 years ago

Do you know if the SWMM repo contains other preprocessing utilities like this one?

Are there any other owa repos that provide preprocessing plugins for SWMM/pyswmm?

If you want to avoid dependencies, I could add a module to digest pyswmm datastructures within pysheds or some other preprocessing repo. However, this would make the preprocessing features less discoverable to users of pyswmm.

lrntct commented 6 years ago

I believe there is a plan to add an API for the input/output of SWMM (can't find the issue, though). The original SWMM is heavily reliant on the text input file. When this change in API is done, it will be much easier to interface SWMM with a GIS, where the network functionalities exist.

jennwuu commented 6 years ago

Not sure if this has been suggested before, what about a qGIS plugin for PySwmm for GIS functionality?

bemcdonnell commented 6 years ago

@mdbartos et al., my thoughts at this point are that we keep pyswmm as the light-weight low level Python api. But that we continue to enable a plugin architecture such that someone could maintain a separate package for other tools. The question then becomes, what functions are base functionality, and what functions are specific-application-feature-driven. If we loosen the scope of the project, sometimes the software can drift into something that is very hard and tedious to maintain.

GIS makes me think about coordinates. I think getters for coordinates for Nodes, Link vertices, and Subcatchment polygons would be nice as a base function. We could also debate how to best implement this (whether we read directly from the inp as needed, or we load it, or we pipe it in through the SWMM API).

More pertaining to what @mdbartos is proposing; PySWMM currently offers base functionality to check link/node/subcatchment connectivity. We haven't adopted any on-board tools with pyswmm at this point. I'm not opposed to it, but I feel that we need to be very careful in terms of scope creep.

What are your thoughts?

lrntct commented 6 years ago

@jennwuu It would be nice, but I think the first step is to get the I/O API to work. If not the plugin would be similar to this one.

aerispaha commented 6 years ago

I'm working on the sewergraph project which is aiming to be a collection of common graph computations relevant to gravity flow networks. I'd love some help over there! This is how I plan to pre-process hydraulic models and do more sophisticated graph calcs.

Currently, I'm mimicking the osmnx API (graph calcs for street networks). This means I'm depending on networkx and the (beefy but awesome) GeoPandas.

dickinsonre commented 6 years ago

Hello @aerispaha wouldn't this be a better function or addition to the SWMMUI or EPANETUI PyQT Gui? https://github.com/USEPA/SWMM-EPANET_User_Interface

aerispaha commented 6 years ago

Hi @dickinsonre, that could be a good addition there. I think it makes sense to maintain this sewergraph project for folks that want to programmatically work with sewer network graphs though. I think sewergraph could be a dependency of GUI software.

As a shout out to a quiet repo: I've used jsnetworkx to create graph-enabled UIs in browsers with success.

dickinsonre commented 6 years ago

Thank for the Felix and React lead, @aerispaha

DJHostetler commented 6 years ago

Late to the party on this one and far from an expert with this crowd, but I agree wholeheartedly with @bemcdonnell on the proposal. You could create a separate companion library that does the geospatial analysis/calculations that will undoubtedly have several additional dependencies, and separately expose the setters/getters of the shape and geometry functions in this project.

lucashtnguyen commented 6 years ago

Also late to the party. My colleague Austin Orr and I figured out the network tracing of SWMM with networkx (https://github.com/austinorr/swmmnetwork) and I'm currently working on a class to get shapefiles to swmm inps (https://github.com/lucashtnguyen/shmm).

swmmnetwork is complete whereas I am hacking together shmm as we speak (and expect to have it more developed with tests, etc, later next week).

These two libs don't exactly accomplish what @mdbartos requested but hopefully they are somewhat useful.

lrntct commented 4 years ago

GIS makes me think about coordinates. I think getters for coordinates for Nodes, Link vertices, and Subcatchment polygons would be nice as a base function. We could also debate how to best implement this (whether we read directly from the inp as needed, or we load it, or we pipe it in through the SWMM API).

@bemcdonnell I also believe that having the coordinates as object properties could be considered a basic feature. I have somewhere a piece of code that parses the INP for links vertices and nodes coordinates. If there is an interest, I could try to adapt it for pyswmm and open a PR. I think it should be easy to do the same for catchment polygons.

Edit: Currently, the swmm input reader does not even care about the coordinates. So it might be easier to do a pure Python option. The swmmio package does that, but adding dependency to Pandas just to read coordinates might be overkill.

dickinsonre commented 4 years ago

So you are saying that currently PySWMM does not store the geometry features and does not export any coordinates to the SWMM5 engine? Not that the engine currently uses and GIS-related information but in the future, it might be nice to have rain gages cover the Subcatchment polygons for example to extract area-weighted rainfall.

bemcdonnell commented 4 years ago

@dickinsonre, PySWMM doesn't really do anything with the INP directly. The API on SWMM exposes the SWMM data model once the INP is parsed and read in. SWMM does not read many sections of the INP including [COORDINATES], [VERTICES], or [POLYGONS]. I am open to talking about exposing these through PySWMM properties but I am not overly excited about it. @aerispaha's swmmio is a great tool that already exposes these things. The use-case for PySWMM and GIS seems a little out of scope. When you need coordinates you are doing plotting, probably. PySWMM is more so opening up the interactive simulation opportunities.

I'm starting feel like my opinion is going to be out-voted. :-) .. and if this is the case, the democratization approach around these open source projects is working haha

lrntct commented 4 years ago

@bemcdonnell As a use case example, coupling a surface model requires knowing the node coordinates. That is why itzi currently includes a swmm inp parser. More generally, whether or not the coordinates are part of the swmm data model is debatable, but I believe it is reasonable to consider that a pyswmm user will expect to have access to every information present in the SWMM INP. This includes the coordinates. Note that I am not arguing about adding any GIS functionality. Simply exposing the coordinates of the objects that are included in the INP file. A Python INP parser is relatively lightweight and I think this could be implemented with less than 500 lines of code.

bemcdonnell commented 4 years ago

Thanks @lrntct. I understand your use case better. Ok! @jennwuu / @katmratliff / @abhiramm7 what do you think? Final concerns are around coordinate systems - do we need to think about this (@aerispaha)?

lrntct commented 4 years ago

@bemcdonnell I would consider the management of CRS out of scope.

See Shapely as a example: no support for CRS. There is geopandas for that.

abhiramm7 commented 4 years ago

@bemcdonnell I agree with you, adding GIS functionality to pyswmm would be out of scope, but having an add on module that provides GIS functionality would be cool.

I really like @lrntct idea of incorporating a lite INP parser into pyswmm. I have been using swmmio to do that and having it as a feature of pyswmm would be good. This parser also can be repurposed to generate/update input files, which could be really useful in designing infrastructure.

jennwuu commented 4 years ago

@lrntct I agree that we should be able to get coordinates from swmm datamodel. Can you explain what functionalities will be provided by Python INP parser? Will this parser be outside of swmm data model?

Can we use functions from swmmio? I know swmmio has functionality to get/modify data from SWMM inp/out/rpt file. @aerispaha

It would also be nice if we can edit SWMM data model via pyswmm and have pyswmm dump out the datamodel back into inp file.

michaeltryby commented 4 years ago

I actually have a more radical idea. I think we should migrate SWMM input data to a geospatial database format. Then develop modules around it to provide the interfaces and services we want. This is what I’m considering for the UI project.

dickinsonre commented 4 years ago

If you do that @michaeltryby then you lose the universal value of the SWMM5 inp file format to so may vendors, Matlab users, and Python programmers in general. Not to mention that the simplicity of the inp file makes it easy to edit and understand.

michaeltryby commented 4 years ago

@dickinsonre i understand the appeal of the input file format. We would still support it with import and export functionality. I encourage you to think instead about what would be gained by using a database to manage data and conforming with OGC standards. It would create exciting new data workflow possibilities.

lrntct commented 4 years ago

@abhiramm7 The embedded lite parser is fine to just expose the coordinates as object properties. If we want a more generic parser / writer, then maybe a separate, generic inp_parser module would be more adequate. Then pyswmm could rely on that module.

I really like the idea of @michaeltryby, for the long term. For the time being, I believe that a light, embedded parser is a "low hanging fruit" approach that could be achieved in a short timeframe and achieve a specific purpose. Later, the functionalities could be extended without breaking the public API.

aerispaha commented 4 years ago

The swmmio project covers parsing the INP file and exposes getters/setters for most of the typically used sections. I'd recommend folks consider that before rewriting another parser.

For example:

model_a = swmmio.Model('path-to-model.inp')

# dataframe of [COORDINATES] section
df = model_a.inp.coordinates 

It also provides a means to deal with coordinate reference systems, and easy export of nodes, links and subcacthments as geosjon (or whatever spatial format you prefer).

So, I definitely think swmmio fits the bill if the use case is getting coordinates from an INP file. But, even though hate I to see the community rewriting INP parsers over and over again, I also understand that Pandas is not a small dependency.

But maybe the calculation is different if we look further into our future plans - if, for example, we have plans to incorporate a spatial-based data model for SWMM in the future, we'll again be confronted with the decision of whether to use Pandas/GeoPandas, which is wonderful, feature-rich, and big, or to rewrite a lot of logic and solve a lot of problems that have been solved before.

Not sure what the best path forward is, I think there are pros/cons to both. Anyone want to decouple the swmmio from Pandas for lighter-duty use-cases?

DJHostetler commented 4 years ago

Not sure what the best path forward is, I think there are pros/cons to both. Anyone want to decouple the swmmio from Pandas for lighter-duty use-cases?

I agree @aerispaha that swmmio would be the answer if not for the pandas dependency. One of the things I like about libraries like shapely and pyswmm is the lack of dependencies (along with a well thought out design).

@michaeltryby On the other front of changing the SWMM input file to a geodatabase, as a user I have mixed feelings. What file type would you use? If you go with shapefiles you are going to be messing with a host of files. If you go with the geopackage (open source geospatial sqlite) format now you have a binary format that has little to no support from ESRI. The biggest benefit to a binary format would be better performance reading/writing for very large models, but that is an edge case IMO.

If we have the ability to read and write the input file format with a commonly used language like Python, do we really need to change from a text file format? If you can read the input file, you can perform geospatial analysis with shapely or gdal. The biggest issue I find with the SWMM text files is that there is no versioning that comes along with the file. I can open HEC-RAS input text files and know what version it was written in.

dickinsonre commented 4 years ago

There is a version number for SWMM5 in the OUT binary file which gives you a clue as to when the INP was last used/run. I agree the lack of version number in the input file is a real drawback for those of us with thousands of inp files or even a decades-long list of input files.

abhiramm7 commented 4 years ago

@gregjewi can swmmWrangler be used as a INP file parser? I am not sure if you had pandas as a dependency or not.

lrntct commented 4 years ago

As an example, here is the basic INP parser that I wrote for getting the nodes and links coordinates:

https://github.com/ItziModel/itzi/blob/2785fbb5e0c041ed4367eb1634d4d1f93bea67c6/itzi/drainage.py#L195

It does not rely on any dependency and I believe it could be easily integrated in pyswmm. It even does too much and could be stripped down.

gregjewi commented 4 years ago

@abhiramm7 no, the parser and the data structures do not rely on pandas. The parser in swmmWrangler has the capability to import sections not listed in the above link, namely: map, polygon, and symbols. (I wanted to do affine transformations with the geo data and needed all the geo data in the inp files I was processing.) As a side, I never found/searched for a canonical INP file to base my parser off of, just used a really thorough inp file to parse. In its current state, it'd need some attention to be a contender for the lite, pandas-free parser proposed here. Not impossible though!

It's been awhile since I was using this day-to-day, but exposing just the coordinate data isn't going to be super helpful without the other geo data, which opens up questions of how to expose those etc... Leads back to @bemcdonnell 's point of scope creep. Iirc, I extended pyswmm in much the way suggested above. The pyswmm link/node objects were stored as attributes of my own link/node objects whose data model included other attributes, such as geo data, that were used for different applications.

michaeltryby commented 4 years ago

This topic has generated some great discussion! Thanks to everyone for sharing their ideas! I've been giving this some thought and perhaps there is an alternative we haven't considered.

The philosophy of pyswmm, which I have discussed many times with @bemcdonnell was to do things that other Python wrappers weren't doing by building out SWMM's C API. This turned out to be a serviceable idea that unlocked a lot of functionality. I think we can apply the same idea here as well.

As as alternative to writing another parser we could simply extend SWMM's existing parser to process the geometric data in the input file and write some C API for access to it. We are only talking about a couple of new sections and adding a few data structures to hold the resulting data. Given the complexity of the C API that has already been delivered as part of OWA SWMM it is well within the capabilities of the team to knock this out. Any volunteers?

In the interest of full disclosure, when the OWA EPANET developers were faced with the same problem during their EPANET v2.2 development campaign this is the solution they opted for and it worked out pretty well.

DJHostetler commented 4 years ago

As as alternative to writing another parser we could simply extend SWMM's existing parser to process the geometric data in the input file and write some C API for access to it. We are only talking about a couple of new sections and adding a few data structures to hold the resulting data. Given the complexity of the C API that has already been delivered as part of this project it is well within the capabilities of the team to knock this out. Any volunteers?

@michaeltryby Are you referring to this effort in the OWA SWMM repository? https://github.com/OpenWaterAnalytics/Stormwater-Management-Model/issues/47

Unfortunately I wasn't able to get clearance at my firm to contribute to that effort, but hopefully someone else can find the time and resources to contribute. What might help get the group effort going is for someone who is more C literate to write out a small part of the input and output parser as a go by.

Edit: The epanet "inpfile.c" is a good go by.

bemcdonnell commented 4 years ago

Thanks everyone who has contributed into this discussion. I think integrating the coordinate features [coordinates] [vertices] [polygons] into SWMM's data model and exposing these through an API will centralize "parsing" to a single parser down at the lowest level. I believe it will be a better building block for ALL potential higher-level wrapping languages (Python, R, MATLAB, ...). So I am going to move this issue over to OWA-SWMM. Once the API functions are exposed over in SWMM, we can talk about wrapping them here.

michaeltryby commented 4 years ago

We are looking for volunteers to help implement this feature. Don't be shy! :smile:

bemcdonnell commented 4 years ago

The task is as simple as taking code from epanet, moving it here and compiling it then writing some unit tests :-) @aerispaha - what are you up to this weekend?

bemcdonnell commented 4 years ago

@cbuahin what are you doing this weekend?