NREL / EnergyPlus

EnergyPlus™ is a whole building energy simulation program that engineers, architects, and researchers use to model both energy consumption and water use in buildings.
https://energyplus.net
Other
1.14k stars 392 forks source link

Results change when fluid caching is used #10029

Closed rraustad closed 8 months ago

rraustad commented 1 year ago

Issue overview

Testing shows that the results of a simulation using plant loops change when fluid caching is used (default). Compiling an executable without fluid caching show different results.

CMake switch: image

image

Details

Some additional details for this issue (if relevant):

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

jmarrec commented 1 year ago

@rraustad isn't that somewhat expected though? Caching speeds things up at the cost of some inaccuracies. Is it the magnitude of the deviation that's of concern here? What's the total EUI deviation you're experiencing here?

rraustad commented 1 year ago

Small diffs would make sense but the diffs seem larger than what I would expect. A difference of 2C-4C is larger than expected. The difference I would expect would be on the order of 0.1C. It may be that the wrong fluid property value is being returned from the cache? Has anyone ever looked at that? Something seems wrong here.

jmarrec commented 1 year ago

This specific file was mentioned during the initial development of the Glycol caching: https://github.com/NREL/EnergyPlus/pull/7361 (see https://github.com/NREL/EnergyPlus/pull/7361#issuecomment-532340251 in particular)

The file is also mentioned in the https://github.com/NREL/EnergyPlus/blob/develop/design/FY2019/Refactor%20and%20cache%20two%20psychrometric%20functions.md Table 3. Regression diffs analysis of the PsyTsatFnPb function.

jmarrec commented 1 year ago

I ran the file with caching and without caching, annual run.

Total Site Energy [GJ]:

jmarrec commented 1 year ago

The Ground Heat Exchanger inlet temperature is oscillating like crazy even without caching enabled.

Here is the annual run.

image

Now, zooming into july 2nd:

You can also see that the "resting" state when the system is off at night changes quite dramatically at the end of the day.

image

Describing

When you look at the statistics, it seems to match up pretty well overall anyways.

image

rraustad commented 1 year ago

I wonder if these same diffs show up with a cooling tower? When I was looking at this a few months ago I was also using a file with a GHX.

jmarrec commented 1 year ago

@rraustad I have started by modifying the file, and I added a hardsized (way oversized CT), and noticed the difference was almost nothing. I then added an autosized CT, and noticed the difference was slightly greater. So I then took the GHX file, and increased the water flow rate in the GHX + PlantLoop maximum flow rate, and the difference is much less.

Test files and graphs can be found at https://github.com/NREL/EnergyPlusDevSupport/tree/master/DefectFiles/10000s/10029

Original CentralChillerHeaterSystem_Cooling_Heating.idf

ghx-all_year

Increased GHX size (CentralChillerHeaterSystem_Cooling_Heating_BiggerGHX.idf)

biggerghx-all_year

NOTICE HOW PCT CHANGE IS 80% in the first case, 8% in the second one

Cooling Tower, autosized (CentralChillerHeaterSystem_Cooling_Heating_WithCT_autosized.idf)

ct-autosized-all_year

Cooling Tower, oversized manually (CentralChillerHeaterSystem_Cooling_Heating_WithCT.idf)

ct-all_year

jmarrec commented 1 year ago

This is the "Total Site Energy [GJ]" for all of the above test cases:

image

I recommend we close as "Works as expected". @rraustad please advise, thanks!

rraustad commented 1 year ago

Can you push up a branch with caching turned off to see what diffs CI finds? I've done that locally but did not save the results.

jmarrec commented 1 year ago

@rraustad i enabled it for the linux regression runner at

https://github.com/NREL/EnergyPlus/commit/afa7d3eec41590798fb641f365241a278c262359#commitcomment-126831930

jmarrec commented 1 year ago

big maths:

['CentralChillerHeaterSystem_Simultaneous_Cooling_Heating',
 'CoolingTower_VariableSpeed_IdealCondEntTempSetpoint',
 '5ZoneCoolingPanelBaseboardAuto',
 'CentralChillerHeaterSystem_Cooling_Heating',
 '5ZoneIceStorage',
 'HAMT_DailyProfileReport',
 'ASHRAE901_RestaurantFastFood_STD2019_Denver',
 'HeatPumpWaterHeater',
 '5ZoneVAV-ChilledWaterStorage-Mixed',
 'ASHRAE901_RestaurantSitDown_STD2019_Denver',
 'HeatPumpWaterHeaterStratified',
 'HeatPumpWaterToAirWithRHControl',
 'PythonPluginReplaceTraditionalManagers_LargeOffice',
 '5ZoneAirCooledWithCoupledInGradeSlab_HorizInsThickness',
 'RefBldgMediumOfficeNew2004_Chicago_JSON_Outputs',
 'HeatRecoveryPlantLoop',
 'CoolingTower_VariableSpeed_IdealCondEntTempSetpoint_MultipleTowers',
 'VSHeatPumpWaterHeater',
 'ASHRAE901_RetailStandalone_STD2019_Denver',
 '5ZoneTDV',
 '5ZoneVAV-ChilledWaterStorage-Mixed_DCV_MaxZd',
 '5ZoneVAV-ChilledWaterStorage-Mixed_DCV_MultiPath',
 'ShopWithBIPVT',
 '5ZoneAirCooled_AirBoundaries',
 'PlantLoopHeatPump_EIR_LargeOffice-2-AWHP-AuxBoiler-Pri-Sec-4PipeBeam',
 'RadLoTempHydrChangeoverDelay',
 'PlantLoopHeatPump_EIR_Large-Office-2-AWHP-DedHR-AuxBoiler-Pri-Sec-HW']

Parsing the total site energy for all of these files

image

can we close @rraustad please?

rraustad commented 1 year ago

@amirroth note the suggested test for diffs is annual energy use instead of the typical CI diffs.

I still wish I understood why the plant lands on a different solution at some points in the simulation with and without caching (not expected) while retaining the same energy use (expected). Anyone care to comment on that?

Myoldmopar commented 1 year ago

Miniscule differences in fluid property lookups causing equipment to cycle at different times? So if you look at it hourly or timesteply, they look like huge changes, but really they are just slight timing differences? Perhaps mathdiff should be updated to include some interesting new comparisons on the time series data?

rraustad commented 1 year ago

This issue was prompted by #9946 diffs which seemed to indicate that the original fluid caching method had a problem. If everyone thinks these results prove that caching is working correctly, then yes this can be closed. And then a new look at #9946 would be the next step.

jmarrec commented 8 months ago

@rraustad Can we close this one now?

rraustad commented 8 months ago

I think so.