Skip to content

Fix #4699 - E+ 22.2: ZoneHVAC:PackagedTerminal AirConditionner/HeatPump requires a supply air fan operating mode schedule#4701

Merged
tijcolem merged 10 commits intodevelopfrom
4699_VT_PackagedSys
Oct 6, 2022
Merged

Fix #4699 - E+ 22.2: ZoneHVAC:PackagedTerminal AirConditionner/HeatPump requires a supply air fan operating mode schedule#4701
tijcolem merged 10 commits intodevelopfrom
4699_VT_PackagedSys

Conversation

@jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Sep 29, 2022

Pull request overview

Changes to ZoneHVACPackagedTerminalAirConditioner and ZoneHVACPackagedTerminalHeatPump

API Break

* `ZoneHVACPackagedTerminalAirConditioner` and `ZoneHVACPackagedTerminalHeatPump` have an API-breaking change related to its `supplyAirFanOperatingModeSchedule` getter. It is now a required field that returns `Schedule` instead of `boost::optional<Schedule>`. Method `resetSupplyAirFanOperatingModeSchedule` is also removed.
    * It is set to `alwaysOnDiscreteSchedule` (=Constant) in the Constructor if you provide a `FanConstantVolume` (This is **required** by E+)
    * It is set to `alwaysOffDiscreteSchedule` (=Cycling) in the Constructor if you provide any other fan types (E+ treats a blank schedule as always off)

Unusual Version Translation rules to preserve functionality and Energy Usage

* There are unusual `VersionTranslator` Rules for Packaged Systems (PTAC or PTHP) that use a `FanConstantVolume` and that do not have a `Supply Air Fan Operating Mode Schedule`. In 22.1.0 this would effectively, and mistakenly, function as a cycling fan, but this is now disallowed in E+ 22.2.0. In order to retain a similar functionality and energy usage, the `FanConstantVolume` will be replaced by a `FanSystemModel` with an Always Off Schedule (=cycling fan, similar to a `Fan:OnOff`), mapping inputs such as pressure rise and efficiency appropriately.

Pull Request Author

  • Model API Changes / Additions
  • Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
  • Model API methods are tested (in src/model/test)
  • EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
  • If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
  • 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.
  • All new and existing tests passes
  • If methods have been deprecated, update rest of code to use the new methods

Labels:

  • If change to an IDD file, add the label IDDChange
  • If breaking existing API, add the label APIChange
  • 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

@jmarrec jmarrec added component - HVAC component - IDF Translation component - Version Translator Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. APIChange A motivated non-backward compatible change that breaks the existing API and needs to be communicated labels Sep 29, 2022
@jmarrec jmarrec self-assigned this Sep 29, 2022
@jmarrec jmarrec changed the title #4699 - E+ 22.2: ZoneHVAC:PackagedTerminal AirConditionner/HeatPump requires a supply air fan operating mode schedule Fix #4699 - E+ 22.2: ZoneHVAC:PackagedTerminal AirConditionner/HeatPump requires a supply air fan operating mode schedule Sep 29, 2022
@jmarrec
Copy link
Collaborator Author

jmarrec commented Sep 30, 2022

Using a custom build of E+ on my fork: https://github.com/jmarrec/EnergyPlus/releases/tag/v22.2.0-WithIHGFix

At this point the only failure I have left is the ems_scott.osm and it is due to ZoneMixing mapping to Spaces and EMS usage.

The EUI deviations are wild, but this is because the tests use a Fan:ConstantVolume, and before we didn't set a schedule so it was working as a cycling fan... bummer. I think I'm going to have to modify the tests / baseline lib to include a Schedule of always 1 and run all reg tests backwards in all OS version... geez.

image

@jmarrec jmarrec force-pushed the 4699_VT_PackagedSys branch from 3dde95c to 46881f6 Compare September 30, 2022 09:01
@jmarrec
Copy link
Collaborator Author

jmarrec commented Sep 30, 2022

I made a modification in NatLabRockies/OpenStudio-resources@9a404be to force an Aways On Supply Air Fan Operating Mode Schedule, and rerun with OS 3.4.0, so I could exclude that.

Looks better.

image

@jmarrec
Copy link
Collaborator Author

jmarrec commented Sep 30, 2022

Diff between 3.4.0 PackagedFanOpSch and 3.5.0:

image

I'm confused by the ptac_othercoils.osm one. Looking into it

@jmarrec
Copy link
Collaborator Author

jmarrec commented Sep 30, 2022

I don't get why ptac_othercoils.osm is overusing by 30%...

@tijcolem
Copy link
Collaborator

tijcolem commented Oct 4, 2022

"I think I'm going to have to modify the tests / baseline lib to include a Schedule of always 1 and run all reg tests backwards in all OS version... geez". We can deal with this post release if this still needs to happen.

So I ran regression tests on tip of develop and seeing 69 failures which I'm assuming all due to the large EUI diffs you highlighted above? I am re-running the windows as I saw some unit test failures but it could be a build cache issue going on with that. @jmarrec I'll check results but providing windows checks out I'm assuming this is good to merge?

@tijcolem
Copy link
Collaborator

tijcolem commented Oct 6, 2022

@jmarrec The units tests all passed on the Windows runner doing a full build so this is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

APIChange A motivated non-backward compatible change that breaks the existing API and needs to be communicated component - HVAC component - IDF Translation component - Version Translator Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.

Projects

None yet

3 participants

Comments