Add detailed pump model#45
Conversation
…ciency curves. Successfull test for RO feed pump.
jakechurchi
left a comment
jakechurchi
left a comment
There was a problem hiding this comment.
Made some comments I can immediately address, but many that require some feedback / discussion
…ow pump speed (50%) test
|
|
||
| @pytest.mark.unit | ||
| def test_low_speed(): | ||
| # Doubles as low speed (50%) test |
There was a problem hiding this comment.
For lower speed while using head and speed, I have found the first initialization needs to be sufficiently close to the final speed (test_pump_speed here) or there may be initialization issues. Idk if there is a standard to note details like that in documentation.
There was a problem hiding this comment.
this is great information and should be reflected with some sort of logic in the initialization routine
There was a problem hiding this comment.
I tried to re-create this issue, but doesn't seem to be a problem anymore? May have been a scaling factor issue. The initial flowrate guess can be pretty off and this test still passes.
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_negative_inlet_pressure(): |
There was a problem hiding this comment.
Documentation in PR #48 should be updated to reflect negative inlet pressures.
There was a problem hiding this comment.
what is the use case for a negative inlet pressure? Do you think it is common enough that changing the lower bound for pressure should just be part of the build?
There was a problem hiding this comment.
Yes, I think it common enough to move in.
Negative pressure is occurs when there is an elevation change the pump must overcome, like with the UF pumps.
| ) | ||
|
|
||
| # Add efficiency variables for more and VFD efficiency. Only defined for variable efficiency | ||
| self.vfd_efficiency = Param( |
There was a problem hiding this comment.
While not needed for WRD, for smaller pumps, vfds, and motors, the efficiency will change with speed more significantly. I image some additional work on this component before merging into WaterTAP.
kurbansitterley
left a comment
kurbansitterley
left a comment
There was a problem hiding this comment.
configuration logic should be clean up a bit and we should implement any custom logic needed in the initialization routine for particular use cases.
| if isinstance(filepath, str): | ||
| try: | ||
| df = pd.read_csv(filepath) | ||
| except Exception as e: |
There was a problem hiding this comment.
would this fail because the file isn't there? If so FileNotFoundError
There was a problem hiding this comment.
and actually, unless you intend to catch some other behavior here or want to have a custom error message, pandas will handle that for you.
There was a problem hiding this comment.
Because it fails during the domain validation, pyomo actually throws a ValueError
| from models.pump_detailed import Pump, Efficiency, PumpCurveDataType | ||
|
|
||
| solver = get_solver() | ||
|
|
There was a problem hiding this comment.
because the CONFIG is so critical to what is built, you should have a test that has invalid config and make sure proper errors are raised (search watertap main for with pytest.raises()...)
There was a problem hiding this comment.
or check out the test for chemical addition in this repo
There was a problem hiding this comment.
I've added a number of these.
|
|
||
|
|
||
| @pytest.mark.unit | ||
| def test_negative_inlet_pressure(): |
There was a problem hiding this comment.
what is the use case for a negative inlet pressure? Do you think it is common enough that changing the lower bound for pressure should just be part of the build?
…r/flex_desal into pr/MuktaHardikar/45
3 participants
This pump model simultaneously solves a system curve and pump curve to calculate the efficiency at the selected design point using affinity laws.