Closed Myoldmopar closed 10 years ago
Raw defect file can be downloaded here.
OK, I have a proposed fix, but not completely settled that it is robust enough just yet. The problem is that the requested demand is getting reset to zero too often.
WaterUse::CalcConnectionsFlowRates()
is called to look for water demands.
VdotRequestDemand
set to 2e-6 m3/s
...good.VdotAvailDemand = 0
)HVACManager
, WaterManager::ManageWaterInits()
is called, which in turn calls WaterManager::UpdateWaterManager
. This does many initializations of the water management system.VdotRequestDemand==0
), it maintains that it has zero available demand to give out (VdotAvailDemand=0
).This is definitely not meant to be a final form of the fix, but the idea is that the initialization needs to be muffied, or delayed. As a test, I commented out the initialization of demand request. The results come out awesome now:
I need to do some testing, as I don't want to cause other issues by commenting that out, but I think it is on the right track.
OK, so the initialization back to zero is really troublesome for this situation because of the order of operations with water use and storage. It seems like initializing that to zero at any point seems to be a bad idea and we should enforce that other components update that value themselves. The storage tank shouldn't be the one to impose a value on the demand requested of it, be it setting it to a calculated value or setting it to zero. I'm going to run a test suite and see if this causes any trouble. If other storage-demand-ish components are obeying by this rule that they always update the value (to zero or otherwise) then this fix should actually be just fine.
OK, looks like this fix could be even more important than I thought. I ran the test suite overnight (annual, daily report variables), and got diffs in a number of files with water use objects. I dug into 5ZoneWaterSystems, and found that the tank that is supposed to be feeding the towers and irrigation wasn't feeding anything. It was all coming from mains water, the tank stayed full after it filled up in the first couple days, and all the inlet flow was overflowing. In my modified code, the tank feeds the tower and irrigation systems, never goes dry so I believe it doesn't use mains water for these, and it only overflows when it is actually full.
Here is the tank volume in both simulations:
I haven't been able to dig through the rest of the files to see if the same issue was present, and I don't have the time to figure out when this went awry. I'll report some more testing later today and hopefully can merge this fix in soon.
@Myoldmopar Ran 5ZoneWaterSystems with v4.0, 6.0, 7.0, 7.1, 7.2, 8.1.0.004, 8.1.0.005, 8.1.0.006, 8.1.0.007 and 8.1.0.008. The problem first appeared in 8.1.0.008. This checkin for WaterManager.f90 is the likely culprit:
38 10/17/2013 4:46:28 PM zs_NoCRs_Brent Griffith EnergyPlus CR 9348, rework timing of inits and water manager calcs
Interesting, it looks like from this commit that the init call used to only come at BeginTimeStepFlag. I might try protecting the demand request initialization with the same flag now instead of just commenting it out altogether, and see if that works.
Reverse DD might catch things like this (i.e., when things need zeroing and are not).
On 8/19/2014 11:56 AM, Edwin Lee wrote:
Interesting, it looks like from this commit https://github.com/NREL/EnergyPlusArchive/commit/b658dd3097ba2ad1b051fd41f2b4977406a8276e that the init call used to only come at BeginTimeStepFlag. I might try protecting the demand request initialization with the same flag now instead of just commenting it out altogether, and see if that works.
— Reply to this email directly or view it on GitHub https://github.com/NREL/EnergyPlusTeam/issues/4387#issuecomment-52655599.
Richard Raustad Senior Research Engineer Florida Solar Energy Center 1679 Clearlake Road Cocoa, FL 32922 Ph: (321)638-1454 http://www.fsec.ucf.edu/en/
Electric Vehicle Transportation Center http://evtc.fsec.ucf.edu/
That's a good point. Thanks @rraustad.
Well - this should have never made it past the diff checks for 8.1.0.008 - 2% hourly diffs in an example file - and other values going from e-05 to zero, so maybe that's why it was overlooked - falsely discounted as "big diffs of small numbers"?
OK, so any initialization of that value back to zero is causing it to misbehave. I think because of the timing of the multiple calls into the water manager, it shouldn't be initialized back to zero by this routine itself. It needs to be updated by all the demands being placed on the tank outside of this routine. I confirmed proper reverse DD for this file. (Though on DDs the demands weren't on, probably something set funny in the water use schedules. So I did reverse runperiods and the results are perfectly matched.) I'm starting to feel better about this proposed fix.
OK OK, I ran an annual test suite for the files containing WaterUse:Storage, zone timestep level results. No diffs in the files, except that the tank is actually working properly now in the 5ZoneWaterSystems input file. In the develop branch, the tank never has an outlet flow. In the fixed branch, the tank volume goes up and down according to demand and supply. I'm ready to call this fixed.
@mjwitte Do you have any questions about this or would you like me to run further testing before it is good to go? I'm antsy to get rid of showstoppers so I can work on External Interface, etc.
@Myoldmopar I've been following this (mostly) and the fix is certainly better than before.
OK, if you want to wait until the CI machines have their way with the branch completely, that's fine. I'll assign the pivotal ticket for your approval.
@Myoldmopar The tank looks like it's working correctly, but the cooling tower outputs are odd (been that way for a while). For much of the time, the tower reports values for both of these output variables:
CENTRAL TOWER:Cooling Tower Storage Tank Water Volume Flow Rate m3/s and CENTRAL TOWER:Cooling Tower Starved Storage Tank Water Volume Flow Rate m3/s
But TANK FOR TOWERS AND LANDSCAPING:Water System Storage Tank Outlet Volume Flow Rate m3/s shows the sum of the above (i.e. the tank thinks it's giving the tower everything it asks for, which it should be, but the tower thinks it's only getting part of its request from the tank). This was happening even before the tank flows got messed up in v8.1.
The tower also reports the starved amount on this output variable CENTRAL TOWER:Cooling Tower Make Up Mains Water Volume m3 That would be fine, except both the starved variable and the mains variable are metered, so the water is being double-counted. From the mtd output:
Meters for 1625,CENTRAL TOWER:Cooling Tower Storage Tank Water Volume [m3] OnMeter=Water:Facility [m3] OnMeter=Water:Plant [m3] OnMeter=HeatRejection:Water [m3]
Meters for 1637,CENTRAL TOWER:Cooling Tower Starved Storage Tank Water Volume [m3] OnMeter=Water:Facility [m3] OnMeter=Water:Plant [m3] OnMeter=HeatRejection:Water [m3]
2 issues: a) The starved and mains output should be zero in this case - all the makeup should be supplied by the tank (which the tank thinks it's doing). b) Either the starved output should not be metered.or the starved amount shouldn't be added to the cooling tower mains volume.
Adding a new test file with the above outputs active. And a spreadsheet showing history of this file. https://github.com/NREL/EnergyPlusDevSupport/tree/master/DefectFiles/4387
Thanks @mjwitte for looking at this.
So for a), it is very strange. The cooling tower gets called multiple times between updates to the tank. This is because the plant is being simulated within its own iteration loop. The tower required makeup water continues to increase during these iterations. Once this is done, the tank gets simulated on the next HVAC iteration, and checks all the demand values and updates and says, 'sure I can do that', but then the next plant iteration comes and the tower wants more. There seems to be a mismatch between available demand flow and actual demand flow on the tank. A better option would be to implement a function in the tank called "CanIHaveThisMuchFlow", and make every component that uses tank water call this function. The tank could then be updated at every call and we wouldn't have this ping-pong issue. However, that's a big job, much bigger than this CR. This may have to be left as-is for this release and re-opened as a bigger task.
For b), the tower sets up two output variables that both point to the same internal variable here. The two variables are named Cooling Tower Starved Storage Tank Water Volume [m3]
and Cooling Tower Make Up Mains Water Volume [m3]
. These are both set up if the tower is SuppliedByWaterSystem
, inherently assuming that if it is starved, it is then made up by mains water. However, if this is inherent in the model, then why require two output variables? I am guessing this is why the value is being counted twice. Does that make sense with what you are seeing?
If that is indeed the problem, my suggestion would be to just modify the call to one of the variables so that they both aren't counted toward the same end use. My suggestion would also be to leave the issue open about the tank starving the tower unnecessarily, as it is a much larger task to address with all the other showstoppers open right now.
@myoldmopar I agree, simply drop the metering of the Starved variable and keep the metering on the mains variable.
On 9/3/2014 11:50 AM, Edwin Lee wrote:
For b), the tower sets up two output variables that both point to the same internal variable here https://github.com/NREL/EnergyPlusTeam/blob/77148646-WaterUseBugFix/src/EnergyPlus/CondenserLoopTowers.cc#L1901-L1902. The two variables are named |Cooling Tower Starved Storage Tank Water Volume [m3]| and |Cooling Tower Make Up Mains Water Volume [m3]|. These are both set up if the tower is |SuppliedByWaterSystem|, inherently assuming that if it is starved, it is then made up by mains water. However, if this is inherent in the model, then why require two output variables? I am guessing this is why the value is being counted twice. Does that make sense with what you are seeing?
@myoldmopar Why does the tower want more on every plant iteration? That sounds wrong. Is something accumulating when it should be resetting?
On 9/3/2014 11:50 AM, Edwin Lee wrote:
So for a), it is very strange. The cooling tower gets called multiple times between updates to the tank. This is because the plant is being simulated within its own iteration loop. The tower required makeup water continues to increase during these iterations. Once this is done, the tank gets simulated on the next HVAC iteration, and checks all the demand values and updates and says, 'sure I can do that', but then the next plant iteration comes and the tower wants more. There seems to be a mismatch between available demand flow and actual demand flow on the tank. A better option would be to implement a function in the tank called "CanIHaveThisMuchFlow", and make every component that uses tank water call this function. The tank could then be updated at every call and we wouldn't have this ping-pong issue. However, that's a big job, much bigger than this CR. This may have to be left as-is for this release and re-opened as a bigger task.
Origin:
Defect file setup:
WaterUse:Storage
named StorageTank1, standalone (no plant loops)WaterUse:Equipment
named WUE01, set to draw throughout the year with scheduleWaterUse:Connections
named WaterLoop01 references StorageTank01 and equipment WUE01Site:Precipitation
schedules rainfall throughout the yearWaterUse:RainCollector
named RainCollector01 and StorageTank01, collecting from RainCollectorSurface01Verified operation:
Site Precipitation Rate
output variableWater System Rainwater Collector Volume
collector output variableWater System Storage Tank Volume
tank output variable.Water Use Equipment Total Volume
output variable shows usageWater Use Equipment Mains Water Volume
output variable shows exact same usage (see below)Issue statement:
Bug verification:
WaterUse:Connection
simulation seems to be ignoring the tank's availability, so that's the most likely culprit.Other issues:
Water Use Equipment Total Volume
andWater Use Equipment Mains Water Volume
. See the lines here. Again I think this is unrelated to the current issue. As far as I can tell, that variable is only a reporting variable and not used in any calculations. I think it is accidentally reporting correctly in this case because the tank is ignored, so the entire water use is coming from the mains, so they are equivalent.