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

Default new CoilCoolingWaterToAirHeatPumpEquationFit fields in forward translator #4999

Closed
mdahlhausen opened this issue Oct 12, 2023 · 4 comments · Fixed by #5000
Closed
Assignees
Labels
Triage Issue needs to be assessed and labeled, further information on reported might be needed

Comments

@mdahlhausen
Copy link
Collaborator

Issue overview

OS 3.7 adds several new fields to the CoilCoolingWaterToAirHeatPumpEquationFit object:

  • Part Load Fraction Correlation Curve Name
  • Nominal Time for Condensate Removal to Begin {s}
  • Ratio of Initial Moisture Evaporation Rate and Steady State Latent Capacity {dimensionless}
  • Maximum Cycling Rate {cycles/hr}
  • Latent Capacity Time Constant {s}
  • Fan Delay Time {s}

The defaults are not aligned between OS object creation and the version translator.

Current Behavior

In the forward translator, the defaults for these new fields are different than the defaults used when updating an older .osm using the version translator.
The result is a regression error:

For OS:Coil:Cooling:WaterToAirHeatPump:EquationFit called 'T SW Apartment ZN PSZ-AC Water-to-Air HP Clg Coil 20kBtu/hr 13.0EER'  'Latent Capacity Time Constant': true model = '60.0', compare model = '0.0'
For OS:Coil:Cooling:WaterToAirHeatPump:EquationFit called 'T SW Apartment ZN PSZ-AC Water-to-Air HP Clg Coil 20kBtu/hr 13.0EER'  'Maximum Cycling Rate': true model = '2.5', compare model = '0.0'

Expected Behavior

Either the OS defaults should change, or the version translator should set the new defaults upon translation to avoid regression errors.

Steps to Reproduce

  1. Create a model with CoilCoolingWaterToAirHeatPumpEquationFit object in OS 3.6.1
  2. Forward translate it with OS develop
  3. Create a model with CoilCoolingWaterToAirHeatPumpEquationFit in OS develop
  4. compare

Context

impacting openstudio-standards regression tests

@mdahlhausen mdahlhausen added the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Oct 12, 2023
@DavidGoldwasser DavidGoldwasser added this to the OpenStudio SDK 3.7.0 milestone Oct 12, 2023
@jmarrec
Copy link
Collaborator

jmarrec commented Oct 16, 2023

First, just to clarify:

  • Our version translator does the same as the E+ IDFVersionUpdater would do
  • Our Ctor defaults these fields to the same as E+ 23.2.0
  • The discrepancy you are seeing is because of that. I reported this new discrepancy to the E+ team when I noticed it when working on the upgrade to 23.2.0-IOFreeze, see this table at made at that time:

image (10)

It seems that:

  • These factor were set by the parent object, which defaulted to these values
    • (It's also possible that some of these values were actually not set by the parent object but defaulting to these values internally, something I've been told but I have no confirmed)
  • But the default behavior for other coils is to generally zero-out latent degradation...

@jmarrec
Copy link
Collaborator

jmarrec commented Oct 16, 2023

Now:

  • I can't touch the VersionTranslator, as this would create a regression. The behavior of a 3.6.1 model should be preserved when updating to 3.7.0
  • Thus, there are only two possible paths:
    • Have our SDK Ctor use a different default than E+. I'm not a huge fan of this idea, generally speaking, because it's a bit confusing, but it's maybe fine. (I'm going to prepare a PR that does this, so we have CI running on it and you can test, we can decide if we keep it or trash it)
    • You, on openstudio-standards, set these fields explicitly. @mdahlhausen is that an option? (I don't know what are the side effects of doing this)

To be perfectly clear (but add to the confusion), What our VersionTranslator does is actually this:

  • If the coil was inside a parent unitary system, basically use the values there or the default of cycling rate=2.5 / latent time = 60.0
  • If not, use 0.0 and 0.0 (like the Ctor currently does).

@jmarrec
Copy link
Collaborator

jmarrec commented Oct 16, 2023

For reference:

$ rg -g '*.cpp'  --max-depth 1 "setLatentCapacityTimeConstant\([\d.]+\)"
CoilCoolingDXVariableSpeed.cpp
743:    ok = setLatentCapacityTimeConstant(60.0);
795:    ok = setLatentCapacityTimeConstant(60.0);

CoilCoolingDXCurveFitOperatingMode.cpp
459:    setLatentCapacityTimeConstant(0.0);

CoilCoolingDXSingleSpeed.cpp
960:    setLatentCapacityTimeConstant(0.0);
1000:    setLatentCapacityTimeConstant(0.0);

CoilCoolingDXMultiSpeedStageData.cpp
628:    ok = setLatentCapacityTimeConstant(0.0);
675:    ok = setLatentCapacityTimeConstant(0.0);

CoilPerformanceDXCooling.cpp
590:    setLatentCapacityTimeConstant(45);
614:    setLatentCapacityTimeConstant(45);

CoilCoolingWaterToAirHeatPumpVariableSpeedEquationFit.cpp
520:    ok = setLatentCapacityTimeConstant(0.0);
558:    ok = setLatentCapacityTimeConstant(0.0);

CoilCoolingWaterToAirHeatPumpEquationFit.cpp
594:      ok = setLatentCapacityTimeConstant(0.0);

jmarrec added a commit that referenced this issue Oct 16, 2023
Maximum Cycling rate and Latent Capacity Time Constant default to old parent unitary values (2.5 and 60.0 respectively), not E+ 23.2.0 default (which zero latent degradation)
jmarrec added a commit that referenced this issue Oct 16, 2023
…t doesn't have a parent Unitary to the model Ctor values
@mdahlhausen
Copy link
Collaborator Author

Thanks @jmarrec. I can make the openstudio-standards fix.

DavidGoldwasser added a commit that referenced this issue Oct 16, 2023
Fix #4999 - Change latent degration constructor defaults for CoilCoolingWaterToAirHeatPump:XXXX
wenyikuang pushed a commit that referenced this issue Oct 17, 2023
Maximum Cycling rate and Latent Capacity Time Constant default to old parent unitary values (2.5 and 60.0 respectively), not E+ 23.2.0 default (which zero latent degradation)
wenyikuang pushed a commit that referenced this issue Oct 17, 2023
…t doesn't have a parent Unitary to the model Ctor values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triage Issue needs to be assessed and labeled, further information on reported might be needed
Projects
None yet
4 participants