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

revise version management #391

Merged
merged 1 commit into from
Jul 19, 2023
Merged

revise version management #391

merged 1 commit into from
Jul 19, 2023

Conversation

karosc
Copy link
Member

@karosc karosc commented May 9, 2023

This PR is to create a discussion around an approach to address #382

The changes in this PR at the time of its submission restore the EPA version management approach (constant int defined in consts.h). The EPA versioning is maintained to define the EPA engine upon which the pyswmm-SWMM code is based. When changes are made in EPA code, they (along with the version definition) will be merged into this fork without updating the version defined in the swmm-solver cmake project.

The swmm-solver version that is defined in the CMakeLists.txt file is repurposed to define the version of the toolkit API. As changes are made to the toolkit api, the swmm-solver version should be incremented accordingly.

The EPA report file header is restored to document the EPA engine version that was used to run the simulation. A second header line is added to the report file to document the pyswmm toolkit version included in the engine used to run the simulation. Restoring the EPA header to the first line of the rpt file has the added benefit of making it compatible with PCSWMM gui software, where as previously the OWA header would prevent the PCSWMM from parsing the rpt file properly.

In addition to these changes, I'd like to discuss revising code annotations in the source code. Previously the SWMM model was defined as OWA SWMM. Since the project has migrated away from OWA, I am wondering if we should rebrand the source code accordingly. I propose instances of "OWA" or "Open Water Analytics" in the source code by replaced with "pyswmm". Thoughts?

Since the rpt file header has changed, regression tests will fail until this new engine is used to generate new benchmark files with the updated rpt header.

@bemcdonnell @abhiramm7 @michaeltryby

src/solver/report.c Show resolved Hide resolved
extern/version.h.in Outdated Show resolved Hide resolved
@karosc karosc force-pushed the develop_version_management branch from 414a397 to 056675e Compare May 12, 2023 19:27
@karosc
Copy link
Member Author

karosc commented May 12, 2023

Thanks @bemcdonnell , I reverted the changes and implemented a TOOLKIT_VERSION variable in version.h.in that is used to write the API version to the RPT file.

My other question regarding the annotation and project org name in the source stands. I am

In addition to these changes, I'd like to discuss revising code annotations in the source code. Previously the SWMM model was defined as OWA SWMM. Since the project has migrated away from OWA, I am wondering if we should rebrand the source code accordingly. I propose instances of "OWA" or "Open Water Analytics" in the source code by replaced with "pyswmm". Thoughts?

@bemcdonnell
Copy link
Member

@karosc do you have plans to finish this concept? I'm hoping we can make a new release of the API and cascade to swmm-toolkit and pyswmm in the next couple weeks. I want to do some testing on this new calibration method that I'm working on :)

@bemcdonnell
Copy link
Member

Also, to address the failed reg tests I am wondering if we can just ask NRTEST to skip some header rows.

@karosc karosc merged commit 7654a79 into develop Jul 19, 2023
@karosc karosc deleted the develop_version_management branch September 2, 2023 21:59
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

Successfully merging this pull request may close these issues.

2 participants