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

Pollutant treatment enhancements. #92

Closed
bemason opened this issue Dec 20, 2021 · 7 comments
Closed

Pollutant treatment enhancements. #92

bemason opened this issue Dec 20, 2021 · 7 comments

Comments

@bemason
Copy link
Contributor

bemason commented Dec 20, 2021

We made modifications to SWMM to allow more flexibility and options when running pollutant treatment and generation processes. Before we can modify pyswmm to enable this functionality, we need to modify swmm-python. This includes adding CIN, REACTORC, and HRT to shared_enum.py and then the tests for these additions in test_solver.py.

@karosc
Copy link
Member

karosc commented Dec 20, 2021

I think this would also require updating the solver submodule to the latest develop branch of SWMM. @michaeltryby, does that sound alright to you? I'd like to help here because I also have a minor addition to fix subcatchment P_EVAP_RATE. However, I'm not so familiar with your improved treatment routines, so it might take me a while to put together a complete pull request.

@bemason
Copy link
Contributor Author

bemason commented Dec 21, 2021

Hi @karosc, we already made the pollutant changes to SWMM. See this pull request for more details: pyswmm/Stormwater-Management-Model#326. From my understanding after talking with @abhiramm7, the changes in that pull request were the only changes needed in SWMM to enable this functionality and now we need to make the changes to swmm-python, but I could be mistaken.

@karosc
Copy link
Member

karosc commented Dec 21, 2021

You are correct, your additions are present in the develop branch of SWMM. However, for swmm-python links to SWMM as a git submodule, pointing to a specific commit in the SWMM git tree. The current submodule points to a commit in history that predates your commits. So not only will you need to made changes to the shared_enum.py and test_solver.py files, but you will also need to update the submodule to point to the latest development branch of SWMM.

@michaeltryby mentioned updating this library with SWMM 5.1.14 in an earlier issue, but I'm not sure where that stands. If you make your python changes, update the submodule, and all tests pass, you should be good.

@karosc
Copy link
Member

karosc commented Jan 6, 2022

@michaeltryby, I tried to update the swmm-solver to the lastest commit in the develop branch of SWMM, but I think swig is not liking the changes you made here

When I try to compile swmm-python using the latest SWMM branch, I get the following CMAKE error:

[ 91%] Built target runswmm
Scanning dependencies of target solver_swig_compilation
[ 92%] Swig compile solver.i for python
/mnt/SSD/swmm-python/swmm-toolkit/swmm-solver/src/solver/include/toolkit.h:35: Error: Syntax error in input(1).
gmake[2]: *** [src/swmm/toolkit/CMakeFiles/solver_swig_compilation.dir/build.make:83: src/swmm/toolkit/CMakeFiles/solver.dir/solverPYTHON.stamp] Error 1
gmake[2]: *** Deleting file 'src/swmm/toolkit/CMakeFiles/solver.dir/solverPYTHON.stamp'
gmake[1]: *** [CMakeFiles/Makefile2:327: src/swmm/toolkit/CMakeFiles/solver_swig_compilation.dir/all] Error 2
gmake: *** [Makefile:171: all] Error 2
Traceback (most recent call last):
  File "/home/karosc/miniconda3/envs/swmmpd/lib/python3.6/site-packages/skbuild/setuptools_wrap.py", line 593, in setup
    cmkr.make(make_args, env=env)
  File "/home/karosc/miniconda3/envs/swmmpd/lib/python3.6/site-packages/skbuild/cmaker.py", line 515, in make
    os.path.abspath(CMAKE_BUILD_DIR())))

An error occurred while building with CMake.
  Command:
    cmake --build . --target install --config Release --
  Source directory:
    /mnt/SSD/swmm-python/swmm-toolkit
  Working directory:
    /mnt/SSD/swmm-python/swmm-toolkit/_skbuild/linux-x86_64-3.6/cmake-build
Please see CMake's output for more information.

I am too green to monkey around with the swig interface files without direction, thoughts on where the issue lies?

@michaeltryby
Copy link
Contributor

Hey @karosc, sorry about being incommunicado. Are you getting the error when you build locally? What system / compiler are you using?

@karosc
Copy link
Member

karosc commented Jan 7, 2022

I was able to resolve the issue last night by defining EXPORT_TOOLKIT in the swig interface file. See #94, which I just now created.

@abhiramm7
Copy link
Collaborator

Addressed in #94

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

No branches or pull requests

4 participants