OpenWaterAnalytics / EPANET

The Water Distribution System Hydraulic and Water Quality Analysis Toolkit
MIT License
273 stars 203 forks source link

Get and set pressure units in toolkit #719

Closed lbutler closed 1 year ago

lbutler commented 1 year ago

Implemented using EN_PRESS_UNITS as an option to EN_getoption and EN_setoption.

EN_PressUnits enum uses EN_PSI,EN_KPA & EN_METERS which matches internal options but differs from epanet-output which uses ENR_MTR.

I added testing to cover all cases I could think of, but it may be more than what is required, so I can reduce it if you prefer.

Fixes #715

LRossman commented 1 year ago

If a user switches pressure units, say from meters to kPa, then shouldn't all network parameter data that were entered in meters be converted to kPa as well. These parameters would include pressure bounds used in PDA models, pressure limits assigned to PRVs and PSVs, and any pressure limits used in simple and rule-based controls.

LRossman commented 1 year ago

Answering my own question (with help from @Mariosmsk's response to issue #720), I believe the only conversion necessary would be for pressures used in rule-based controls as all other pressure values are already stored in EPANET's internal pressure head units of feet. (Someone please confirm this.) Since the EN_setflowunits function also doesn't consider unit conversions needed for rule-based controls any code to address this issue should also apply to it as well (assuming I'm correct on this point).

lbutler commented 1 year ago

Good catch @LRossman, if someone else doesn't do it I can confirm later in the week.

I assume since most other conversions are handled, we would not need to create a new API for conversions. Instead, create an internal function to deal with this case when EN_setflowunits was called or the pressure units were changed between kPa/meters.

If we confirm its needed and that's the approach we're going with I can also update the PR later this week too.

LRossman commented 1 year ago

Correct, no new API conversion feature is needed (I'm going to close issue #720 shortly). Just a new function in rules.c to handle the units conversion for condition clause variables.

lbutler commented 1 year ago

I made some changes to the code so that when users change the units of flow or pressure, the units in the rules will also update. To do this, I created a function that converts the units in the rules when the user changes the units using the toolkit.

Another option would be to change the way the rules work so that they use the internal units instead, but that would require a lot of changes to the code, so I decided to go with the simpler option for now.

The following changes were made in the latest commit:

There is a bit of code added here, please provide any comments, and nothing is too pedantic. I'm happy to modify anything to match the current style/convention.

LRossman commented 1 year ago

@lbutler good work. I don't see any issues with your code changes so I will merge the PR.