Wrap Coil:Cooling:DX:SingleSpeed:ThermalStorage

[joseph-robertson]

Pull request overview

• Fixes #5055.
• Questions / issues:
  • Inconsistencies in autosizable vs autocalculatable; change openstudio idd to be consistent? (Appears perhaps E+ doesn't make distinction between "autosize" and "autocalculate"?)
    • Update: I changed all autocalculatable to autosizable.
  • Ctor should follow RetailPackagedTESCoil.idf? Or a mix of the idf values and idd defaults?
    • Update: Ctor generally follows idd defaults, with a few exceptions where it uses values found in RetailPackagedTESCoil.idf.
  • Make all non curve fields required so that, even when Mode=No, the fields are pre-populated in case Mode is switched to Yes?
    • Update: No, not taking this approach. All modes are set to No in the Ctor and no curves are populated; it is up to the user to provide all the required curves for a given mode when set to Yes.

Pull Request Author

• [x] Model API Changes / Additions
• [x] Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
• [x] Model API methods are tested (in src/model/test)
• [x] EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
• [x] If a new object or method, added a test in NREL/OpenStudio-resources: https://github.com/NREL/OpenStudio-resources/pull/206
• [ ] If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
• [ ] Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
• [x] All new and existing tests passes
• [ ] If methods have been deprecated, update rest of code to use the new methods

Labels:

• [x] If change to an IDD file, add the label IDDChange
• [ ] If breaking existing API, add the label APIChange
• [x] If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

• [ ] Perform a Code Review on GitHub
• [ ] Code Style, strip trailing whitespace, etc.
• [ ] All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
• [ ] Labeling is ok
• [ ] If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• [ ] If feature, test running new feature, try creative ways to break it
• [ ] CI status: all green or justified

[joseph-robertson]

@shorowit pointed out that it's not always appropriate to use idf testfile values. @DavidGoldwasser pointed out that there may be helpful curve information on other DX coil objects.

[jmarrec]

Make all non curve fields required so that, even when Mode=No, the fields are pre-populated in case Mode is switched to Yes?

I'm kinda torn on this one. There are three options:

• Indeed make them required, do not translate them if the choice field is No
  • Pros: easier for the user
  • Cons: that's 5 extra curves for each of these... model bloats
• Make a model-time bool validityCheck() method, and call that at the beginning of the FT or something similar (this is what's done for the ElectricLoadCenterDistribution, since dependending on some choice fields you must have an inverter or not for eg). Then issue a loud warn, and do not translate it.
  • Pros: identifies the issue (provided the user actually reads the warnings)
  • Cons: could potentially not be translated without the user realizing
• Leave them optional, check nothing, let E+ crash (I assume E+ actually crashes)
  • Pros: implementation is easier
  • Cons: problem is detected much later

Worth noting that I doubt this object will be commonly used by people, so factor that in.

@kbenne Can you please advise here?

[joseph-robertson]

My vote is leave the curves as optionals (so either bullet 2 or 3); it doesn't look the one testfile on the E+ repo with Coil:Cooling:DX:SingleSpeed:ThermalStorage has all the curve fields populated, so what would we set them to?

[joseph-robertson]

I don't see where ElectricLoadCenterDistribution's validityCheck even gets called?

[jmarrec]

I don't see where ElectricLoadCenterDistribution's validityCheck even gets called?

It isn't, but it should.

https://github.com/NREL/OpenStudio/blob/1ce97f5651ff868500e4168aaa8e0a441559edce/src/energyplus/ForwardTranslator/ForwardTranslateElectricLoadCenterDistribution.cpp#L132

[jmarrec]

@joseph-robertson One failing test

1056: [ RUN      ] ModelFixture.CoilCoolingDXSingleSpeedThermalStorage_clone
1056: /srv/jenkins/openstudio/docker-volumes/ubuntu-2004/src/model/test/CoilCoolingDXSingleSpeedThermalStorage_GTest.cpp:992: Failure
1056: Expected equality of these values:
1056:   0
1056:   model.getModelObjects<Curve>().size()
1056:     Which is: 3
1056: /srv/jenkins/openstudio/docker-volumes/ubuntu-2004/src/model/test/CoilCoolingDXSingleSpeedThermalStorage_GTest.cpp:993: Failure
1056: Expected equality of these values:
1056:   0
1056:   model.getConcreteModelObjects<CurveBiquadratic>().size()
1056:     Which is: 1
1056: /srv/jenkins/openstudio/docker-volumes/ubuntu-2004/src/model/test/CoilCoolingDXSingleSpeedThermalStorage_GTest.cpp:994: Failure
1056: Expected equality of these values:
1056:   0
1056:   model.getConcreteModelObjects<CurveQuadratic>().size()
1056:     Which is: 1
1056: /srv/jenkins/openstudio/docker-volumes/ubuntu-2004/src/model/test/CoilCoolingDXSingleSpeedThermalStorage_GTest.cpp:995: Failure
1056: Expected equality of these values:
1056:   0
1056:   model.getConcreteModelObjects<CurveTriquadratic>().size()
1056:     Which is: 1
1056: [  FAILED  ] ModelFixture.CoilCoolingDXSingleSpeedThermalStorage_clone (10 ms)
1056: [----------] 1 test from ModelFixture (11 ms total)
1056: 
1056: [----------] Global test environment tear-down
1056: [==========] 1 test from 1 test suite ran. (11 ms total)
1056: [  PASSED  ] 0 tests.
1056: [  FAILED  ] 1 test, listed below:
1056: [  FAILED  ] ModelFixture.CoilCoolingDXSingleSpeedThermalStorage_clone

[ci-commercialbuildings]

CI Results for faffcfd8b8a99621ca57752e0a741299701502d0:

• Ubuntu 20.04:

• Ubuntu 22.04:

  • OpenStudio-3.8.0+faffcfd8b8-Ubuntu-22.04-x86_64.deb
  • OpenStudio-3.8.0+faffcfd8b8-Ubuntu-22.04-x86_64.tar.gz

• Darwin:

• Windows:

  • OpenStudio-3.8.0+faffcfd8b8-Windows.exe
  • OpenStudio-3.8.0+faffcfd8b8-Windows.tar.gz

• Code Coverage: