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

"Emitter Backflow" Option Incompatible with EPA's EPANET GUI #780

Open
LRossman opened this issue Apr 10, 2024 · 9 comments
Open

"Emitter Backflow" Option Incompatible with EPA's EPANET GUI #780

LRossman opened this issue Apr 10, 2024 · 9 comments

Comments

@LRossman
Copy link
Collaborator

We want the existing EPA EPANET GUI to work with the dev branch (2.3) and its input files. However when the newly added "Emitter Backflow" option keyword appears in an input file the GUI thinks its actually the "Emitter Exponent" option since it only checks the first keyword and it doesn't know about the new backflow option. It will therefore assign its YES/NO value to the emitter exponent property in the GUI. As a result when the GUI tries to run such a project it returns with an input error since the EPANET engine doesn't see a numerical value for the emitter exponent. Since changing the GUI is beyond the scope of this GitHub project we need to change the "Emitter Backflow" keyword to something else. Two possibilities are "Backflow" or "Backflow Emitter". Does anyone have a better suggestion?

NOTE: After the emitter backflow option keyword is changed and it is included in an EPANET input file the GUI will gracefully ignore it when the file is opened and issue a warning message about an unrecognized keyword. The file will otherwise be accepted and will run successfully (assuming there are no other errors) but, of course, without including the option to not allow emitter backflow.

@lbutler
Copy link
Contributor

lbutler commented Apr 10, 2024

@LRossman I added support for the Emitter Backflow open in the epanet-gui repo.

You can check out the PR here:
OpenWaterAnalytics/epanet-gui#16

And there are binaries here:
https://github.com/OpenWaterAnalytics/epanet-gui/releases

The GUI in its current form has almost all the features of the dev branch, the only thing that is outstanding is updating PCV support to be percentage instead of fractional.

@LRossman
Copy link
Collaborator Author

@lbutler I still think we should design the 2.x versions so the "official" EPA-EPANET GUI can still work with them (minus the new features). Changing the "Emitter Backflow" keyword to something else in your OWA GUI code will require minimal effort, yes?

@lbutler
Copy link
Contributor

lbutler commented Apr 10, 2024

That makes sense to me and yes it will be minimal work to update the GUI code.

It would technically cause an incompatibility with .net files between development versions but I'm purposely not supporting that avoid this issue.

@LRossman
Copy link
Collaborator Author

PR #788 has changed the "EMITTER BACKFLOW" option keyword to "BACKFLOW ALLOWED" to resolve the issue raised here.

@lbutler you should probably update your OWA GUI code accordingly.

@lbutler
Copy link
Contributor

lbutler commented May 16, 2024

@LRossman could there be any confusion with the new keyword as I've always consider backflow to be any type of reversing flow?

No great suggestion from me but maybe a few options:

Backflow(able) Emitter
Return(able) Emitter
Reverse(able) Emitter
Dualflow Emitter

Whenever the PR gets accepted I'll make sure to update the OWA GUI code

@LRossman
Copy link
Collaborator Author

How about EMITTER_BACKFLOW? I tested it with the current 2.2 GUI and it simply ignores it - no error message and no confusing it with EMITTER EXPONENT.

@lbutler
Copy link
Contributor

lbutler commented May 28, 2024

@LRossman introducing an underscore to the options where spaces were used previously seems like it would be more confusing than just the option of BACKFLOW ALLOWED

Maybe others have stronger feelings? I won't be a blocker though on whatever is decided

@LRossman
Copy link
Collaborator Author

I'm staying with BACKFLOW ALLOWED. I just merged its PR into dev.

@lbutler
Copy link
Contributor

lbutler commented May 29, 2024

Sorry about all the extra noise in the end! I'll update OWA GUI code with the new keyword

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

2 participants