Checks

• I added a descriptive title to this issue
• I have searched (Google, GitHub) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug Description

Larger numbers in the EPANET report file are sometimes merged, meaning that the blank space between individual values is dropped (e.g., "12345 -12345" becomes "12345-12345"). The ReportFileReader expects regular numerical values and can't deal with this, which leads to an exception being raised.

Steps to Reproduce

Will be added later.

Expected Behavior

Parsing the EPANET results should not lead to an exception.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

Description

Currently, most component attributes, options, report parameter settings, ... default to None. These attributes are in turn not written to input files, when running a simulation. EPANET interprets the missing attributes by using default values instead (e.g., no demand at a Junction equals a demand of 0 in EPANET). This is not very transparent for users since they have to know the default values used by EPANET. Furthermore, using meaningful default values would lead to simpler Writer functions (no need to check if attr is not None, when using default values).

Proposed Solution

Setting explicit default values for all attributes, where default values make sense (e.g., not for Junction demand patterns).
This affects:

• All Network Components
• General settings
• Time settings
• Report parameter and parameter precision settings

Tasks

• Replace None with EPANET default values

Feature context

The current CI workflow for building the package and releasing it to PyPI is based on python-semantic-release v7. This has since been updated to v8 which does not push the package to PyPI or the GitHub registry anymore. This is now accomplished by dedicated actions that also use a different authentication method for PyPI. Also, setup.cfg is not supported anymore for configuring semantic-release.

Proposed Solution

• Migrate from setup.cfg to pyproject.toml as this is the current way of configuring Python projects
• Update build.yml based on this guide
• Add publisher on PyPI for authentication
• Add Python 3.11 to testing matrix

Alternatives

For now, the python-semantic-release version has been set to v7.34.6 (the last v7 version) instead of master, so for now the workflow should still work until the package has been adapted.

Description

Currently, OOPNET doesn't provide any meaningful output. To improve debugging and enable OOPNET to create logs, logging shall be implemented. This would allow applications to include OOPNET in their logging handlers.

Tasks

• Define what should be monitored and at which level
• Add loggers in appropriate modules

Description

EPANET raises warnings for certain events (e.g., negative pressure). These warnings would come in handy if they were raised in OOPNET as well.

Tasks

• Implement warnings either as generic warnings that are printed to the console and logged or individual warnings per warning type similar to how error are handled.
• Adapt report reader

Description

OOPNET provides an API via the api.py file, where all classes and functions are directly imported:
https://github.com/steffelbauer/oopnet/blob/4d44a3669069137c6e1f1bdffd3f3a72f7996d43/oopnet/api.py
A more elegant solution would be to use the __init__.py file in all modules and submodules to make parts of the corresponding modules importable.

Proposed Solution

Using __init__.py files to declare importable functions. This way, not the entire api.py has to be imported, but also specific parts can be used.

Getters can now be imported as from oopnet.utils.getters import * or just import oopnet.utils.getters instead of having from oopnet.utils.getters.element_lists import get_pattern_ids, ... (note: from oopnet.utils.getters.element_lists import * would be bad practice since this would import absolutely everything from those files, including package imports etc.).

oopnet.utils.getters is then imported by the __init__.py file in the root directory

To import OOPNET in other scripts, there are three possible ways:

1. import oopnet: To call get_pipe(), use oopnet.get_pipe()
2. import oopnet as oop: oop.get_pipe()
3. from oopnet import *: Same behaviour as before (get_pipe())

Options 1 and 2 would be more pythonic, Option 3 would be shorter when writing code but maybe less readable for other users.

Tasks

• Create suitable __init__.py structure
• Remove api.py
• Remove 3rd party packages from __init__.py

Description

Currently, simulation results are read from the report files created by EPANET. To improve performance, the binary out files should be read instead.

Tasks

• Write binary file reader
• Write unittests
• Switch from report file reading to binary file reading in ModelSimulator

Description

There are two issues with the current Valve implementation:

1. The Valve's type is explicitly included as a Valve attribute, although the correct Valve subclasses are used.
2. GPVs currently don't hold a reference to the corresponding Curve but instead uses the Curve's ID.

Tasks

• Remove valve_type attribute?
• Switch GPVs to reference Curves directly instead of Curve IDs?
• Rename settings attribute depending on exact Valve type

Description

Sometime, it would come in handy to construct a graph with weight links based on simulation results (e.g., flow, headloss, ...). A possible way of implementing this would be to support pandas Series as weights arguments when calling Graph, DiGraph or MultiGraph. Furthermore, it would be nice to be able to switch the link direction in directed graphs, if a weight is < 0. Users should be able to enable this function by using an argument in the DiGraph and MultiDiGraph calls.

Tasks

• Add support for weights
• Implement a function to switch length directions for directed graphs

Description

Errors raised by EPANET are written to the created report files. Parsing them would enable OOPNET to catch and react to these errors.

Proposed Solution

Since multiple errors could occur in a single simulation, catching an error should not instantly raise an Exception. Errors should be collected first (e.g., by a manager object), and when all errors are parsed, a single Exception containing all the raised errors should be raised. All possible EPANET errors could be implemented as individual exceptions.

Tasks

• Implement individual EPANET errors as Python objects
• Implement a container Class to store all raised errors
• Adapt report reader to read errors from report file

Bug Description

When trying to extract Chemical data from a report of a run with the option Quality=chemical, the code gives the following error:
Traceback (most recent call last):
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\pandas\core\indexes\base.py", line 3791, in get_loc
return self._engine.get_loc(casted_key)
File "index.pyx", line 152, in pandas._libs.index.IndexEngine.get_loc
File "index.pyx", line 181, in pandas._libs.index.IndexEngine.get_loc
File "pandas_libs\hashtable_class_helper.pxi", line 7080, in pandas._libs.hashtable.PyObjectHashTable.get_item
File "pandas_libs\hashtable_class_helper.pxi", line 7088, in pandas._libs.hashtable.PyObjectHashTable.get_item
KeyError: 'Quality'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\xarray\core\indexes.py", line 772, in sel
indexer = self.index.get_loc(label_value)
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\pandas\core\indexes\base.py", line 3798, in get_loc
raise KeyError(key) from err
KeyError: 'Quality'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "C:\Users\Laenenb\PycharmProjects\Epanet\main.py", line 36, in
print(rpt.quality)
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\oopnet\report\report.py", line 114, in quality
return self._get(self.nodes, "Quality")
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\oopnet\report\report.py", line 54, in _get
data = array.sel(vars=var).to_pandas().sort_index()
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\xarray\core\dataarray.py", line 1615, in sel
ds = self._to_temp_dataset().sel(
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\xarray\core\dataset.py", line 3098, in sel
query_results = map_index_queries(
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\xarray\core\indexing.py", line 193, in map_index_queries
results.append(index.sel(labels, **options))
File "C:\Users\Laenenb\PycharmProjects\Epanet\venv\lib\site-packages\xarray\core\indexes.py", line 774, in sel
raise KeyError(
KeyError: "not all values found in index 'vars'. Try setting the method keyword argument (example: method='nearest')."

Steps to Reproduce

Changing line 114 in report.py 'return self._get(self.nodes, "Quality")' in 'return self._get(self.nodes, "Chemical") solved the problem.

Description

Some files in OOPNET seem to have not been used for some time. Some of them do not appear to be working at the moment.

Proposed Solution

Decide which parts of code to keep and refactor them (maybe move them to suitable classes). Get rid of the rest.

Affected files and code fragments

• special_getters.py (self used as function arguments)
  https://github.com/steffelbauer/oopnet/blob/4d44a3669069137c6e1f1bdffd3f3a72f7996d43/oopnet/utils/getters/special_getters.py
• adddummyjunction(), length() and sources() in utils.py
  https://github.com/steffelbauer/oopnet/blob/4d44a3669069137c6e1f1bdffd3f3a72f7996d43/oopnet/utils/utils.py#L26
  https://github.com/steffelbauer/oopnet/blob/4d44a3669069137c6e1f1bdffd3f3a72f7996d43/oopnet/utils/utils.py#L84
  https://github.com/steffelbauer/oopnet/blob/4d44a3669069137c6e1f1bdffd3f3a72f7996d43/oopnet/utils/utils.py#L98
• Report class in pandasreport.py (also see Issue #12)
  https://github.com/steffelbauer/oopnet/blob/4d44a3669069137c6e1f1bdffd3f3a72f7996d43/oopnet/report/pandasreport.py#L9

Tasks

• Implement split() method for Pipes, that takes a split_ratio argument (replaces adddummyjunction())
• Implement get_inflows() and get_inflow_ids() that only returns
  1. Tanks
  2. Reservoirs
  3. Junctions with a demand <0
• Remove legnth() function (it only makes sense with certain coordinate systems/projections)
• Remove getsourcelist()
• Refactor functions in special_getters.py and rename file to something more meaningful

Description

The Report class in xrayreport.py at the moment only serves as a factory for tuples of node and link simulation results. Functions like flow(report) are then used to get specific simulation results. The Report class in pandasreport.py doesn't seem to be used at all.

Proposed Solution

A general Report class could be used for instantiating Report objects. The result getter functions could then be added to the class as properties or methods. Retrieving results as xarray or pandas objects could therefore be unified into a single class.

Tasks

• Create classic Report class from the current factory (data shall be stored in _node_results and _link_results attributes)
• Add methods or properties for getting flow/pressure/... from the stored data

I will fill out the context information later

Feature context

A clear and concise description of what the problem is. E.g.: I'm always frustrated when [...]

Proposed Solution

A clear and concise description of what you want to happen. If possible, please provide a code snippet to demonstrate your solution using code blocks like the one shown below.

print("Hello world!")

Alternatives

A clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context or screenshots about the feature request here.

Checks

• I added a descriptive title to this issue
• I have searched (Google, GitHub) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug Description

Using the plot() method return two subplots.

Steps to Reproduce

network.plot()
network2.plot(nodes=p.iloc[1], links=f.iloc[1], fignum=0, robust=True)

Expected Behavior

If this is the default of plot(), how will the second plot be removed?

Screenshots

Additional context

Description

There are a few minor things that have to/should be done prior to setting this repository to public.

Tasks

• Update setup.cfg
• Remove benchmark.py

Feature context

I believe that the tool can have a bit more of OOPness if the get_ functions (e.g. on.get_junction) are methods of the Network class. As such, we will be using syntax like

network.get_junction()

instead of

on.get_junction(network)

Proposed Solution

Include the get functions into the Network class via wrapper of the already implemented functions

Checks

• I added a descriptive title to this issue
• I have searched (Google, GitHub) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug Description

vertices missing using bokehplot()

Steps to Reproduce

output_file('bokehexample.html')
plot = network2.bokehplot(nodes=p, links=f, colormap=dict(node='viridis', link='cool'))
show(plot)

Expected Behavior

vertices should be reflected (same output of plot())

Screenshots

Additional context

Add any other context about the problem here.

Description

There are several issues with Graphs at the moment:

1. Graphs and DiGraphs don't allow for parallel edges. When converting a network with parallel pipes into a Graph, NetworkX doesn't raise an exception when these parallel pipes are added and only keeps the edge added last (supposedly, didn't check).
2. MultiGraphs allow for parallel edges but are undirected graphs, only MultiDiGraphs would allow for directed edges.
3. The difference in handling edges between MultiGraphs and (Di)Graphs leads to more complex helper functions (e.g., edge2pipeid, edgeresult2pandas). They would have to be redone to be compatible with both (Di)Graphs and MultiGraphs.

Discussion

1. Do Graph and DiGraph classes offer advantages over a MultiGraph? If yes, how should parallel pipes be handled?
2. MultiDiGraphs could be easily added to OOPNET. When considering issue #9, it might make sense to revert edge directions for edges with negative weights when creating MultiDiGraphs.

Proposed Solution

Getting rid of (Di)Graphs, adding a MultiDiGraph factory instead.

Tasks

• Check helper functions for DiGraphs and MultiGraphs
• Implement optional edge direction argument to Graph factories

Checks

• I added a descriptive title to this issue
• I have searched (Google, GitHub) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug Description

When simulating and extended period hydraulic model the resulting report is in data frame format. This is good, however, the index is not sequentially arranged. For example the model runs 24 hours, the report starts from zero then the next data is on the tenth hour.

Steps to Reproduce

rpt = network2.run()
p = rpt.pressure
p

Expected Behavior

the year-month-date format should be removed (perhaps) so the report is arranged hourly.

Screenshots

Additional context

Description

Some of the classes, functions and methods are missing a proper documentation (especially class attributes). This documentation shall be added. Furthermore, the README.md shall be filled with basic content, including a basic description and a link to the docs.

Tasks

• Add module and class documentation to
  • elements (Network, Network Components, ...)
  • graph
  • plotter
  • reader
  • report
  • simulator
  • utils
  • writer
• Sphinx docs
  • Update general OOPNET description
  • Refactor from example-based to topic-based docs
  • Use Jupyter notebooks
  • Update setup instructions
  • Check sphinx docs for completeness
  • Add static bokeh plot example to Latex output of the plotting section in the user guide
  • Add references for models used in the documentation and the corresponding example scripts
• Fill README.md with content

Description

The Sphinx docs should be available online.

Tasks

• Host the docs on ReadTheDocs

Checks

• I added a descriptive title to this issue
• I have searched (Google, GitHub) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug Description

The Model is not able to open a model that has a tank with an overflow in it. The reason seems to be that once that the model takes an overflow, it creates an extra column (Overflow), and gives the volume curve (VolCurve) a placeholder (*). This behaviour can be seen in OWA (http://wateranalytics.org/EPANET/_tanks_page.html)

When the read_network_components.TankFactory is trying to read the tanks, then the overflow column takes column 9 position, and creates a volume curve with the (*) placeholder. As the TankFactory is hard-coded to receive only 8 columns, it triggers a LenghtExceededError (read_network_components.py:140).

Steps to Reproduce

The steps are presented below

EPANET EXAMPLE MODEL

EXAMPLE.inp

Change the extension to INP as GitHub wont allow to upload files with such extension.
EXAMPLE.txt

import oopnet as on

# Load the EPANET model
fname = "EXAMPLE.inp"
net = on.Network.read(fname)

Expected Behavior

It is expected to load the model as a network object.

Screenshots

Feature context

Vertices are only plotted for pipes but not for pumps and valves. These are just straight lines from start to end. This came up when fixing issue #36.

Proposed Solution

Pump and valve vertices should be plotted. Plotting the markers for these objects will need a simple algorithm for calculating their coordinates.

Alternatives

None.

Additional context

Add any other context or screenshots about the feature request here.

Description

Vertices can be used to create better visual representations of the model. Currently, vertices are not read from input files, nor are they completely implemented in the OOPNET.

Tasks

• Add a vertices attribute to Link
• Implement a reader function
• Implement a writer function
• Adapt PlotSimulation() to plot links with vertices
• Add tests and docs

Description

To minimize the effort needed for pushing a new OOPNET version, a CI/CD pipeline would be helpful to automatically:

1. Run tests
2. Reformat code using black
3. Build a new Python package and push it to a repository
4. Build a new docs version and push to host
5. Automatically determine new version number and create changelog (optional: requires semantic-release or something similar)

Tasks

• Decide on CI/CD pipeline (e.g., GitHub actions)
• Decide whether to use semantic-release and automatic changelog creation or other available plugins for semantic-release
• Implement CI/CD Pipelines

Description

All refactoring happened in the dedicated refactoring branch. For publishing, it is necessary to merge this branch into the main branch. The old main branch (Python3 in Gitlab) should still be available as well.

Tasks

• Create new branch from main
• Merge refactor into main

Description

To simplify OOPNET's usage, the read, run and write functions/factories should be refactored so that they are Network class/instance methods.

Tasks

• Refactor read, write and run

Description

Unit tests for several parts of OOPNET would come in very handy while developing it.

Tasks

Add tests (at least) for:

• Controls and rules
• Water quality modelling

Description

The current reader implementation is functional, but a bit messy. Some parts lack structure and are rather complex.

Proposed Solution

• Network Components could be rewritten as shown here
  https://github.com/steffelbauer/oopnet/blob/752c1e31b5c17f570065205912a00b3183ab6c8e/oopnet/reader/read_network_components.py#L32

Tasks

• Decide on new structure
• Refactor Reader

Checks

• I added a descriptive title to this issue
• I have searched (Google, GitHub) for similar issues and couldn't find anything
• I have read and followed the docs and still think this is a bug

Bug Description

When the id of an element starts with a number and there is a - in the name, there seems to be an error when mapping the value as Float. As id's in EPANET are regarded as strings, this behaviour will break if an element has an id such as 1-p. This seems to be related to the reportfile_reader.lst2xray, that is trying to parse the element id's while reading the file.

Steps to Reproduce

Importing the following model will trigger an error

Named_id_model.txt

import oopnet as on

# Load the EPANET model
fname = "Named_id_model.txt"
net = on.Network.read(fname)

Expected Behavior

It is expected that the id's are not parsed, as they are supposed to be strings.

Description

To simplify the usage of OOPNET, hosting it on a Python package repository (PyPI) would be beneficial.

Tasks

• Create shared account or find other possibility to share projects
• Push OOPNET to PyPI