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

Fix #4675 - E+ 22.2.0 - Sizing:Zone has new fields #4677

Merged
merged 13 commits into from Sep 20, 2022

Conversation

jmarrec
Copy link
Collaborator

@jmarrec jmarrec commented Sep 12, 2022

Pull request overview

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

Copy link
Collaborator

@joseph-robertson joseph-robertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want something on OS-resources that exercises these new methods?

@@ -147,6 +150,29 @@ namespace model {

bool isDedicatedOutdoorAirHighSetpointTemperatureforDesignAutosized() const;

std::string zoneLoadSizingMethod() const;
bool isZoneLoadSizingMethodDefaulted() const;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to go with the \required-field and then no isXXXDefaulted or resetXXX approach here?

@joseph-robertson joseph-robertson added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Sep 12, 2022
Comment on lines 25603 to 25680
A8, \field Zone Load Sizing Method
\note Specifies the basis for sizing the zone supply air flow rate.
\note Zone latent loads will not be used during sizing only when
\note Zone Load Sizing Method = Sensible Load Only No Latent Load.
\note For this case the zone humidity level will float according to
\note the fields Cooling and Heating Design Supply Air Humidity Ratio.
\note For all other choices the zone humidity level will be controlled.
\note Sensible Load will use zone sensible air flow rate for zone
\note component sizing. Latent loads will also be reported during sizing.
\note Latent Load will use zone latent air flow rate for zone
\note component sizing. Sensible loads will also be reported during sizing.
\note Sensible and Latent Load will use the larger of sensible and
\note latent load to choose air flow rate for zone component sizing.
\note Sensible Load Only No Latent Load or leaving this field blank
\note will disable zone latent sizing and reporting. Latent loads will
\note not be reported during sizing (reported as 0's).
\type choice
\key Sensible Load
\key Latent Load
\key Sensible And Latent Load
\key Sensible Load Only No Latent Load
\default Sensible Load Only No Latent Load
A9, \field Zone Latent Cooling Design Supply Air Humidity Ratio Input Method
\note Use SupplyAirHumidityRatio to enter the humidity ratio when zone dehumidification
\note is required. The supply air humidity ratio should be less than the zone humidity
\note ratio at the zone thermostat and humidistat set point condition.
\note Use HumidityRatioDifference to enter the difference in humidity ratio from the
\note zone thermostat and humidistat set point condition.
\type choice
\key SupplyAirHumidityRatio
\key HumidityRatioDifference
\default HumidityRatioDifference
N19, \field Zone Dehumidification Design Supply Air Humidity Ratio
\note Zone Dehumidification Design Supply Air Humidity Ratio is only used when Zone Latent
\note Cooling Design Supply Air Humidity Ratio Input Method = SupplyAirHumidityRatio.
\note This input must be less than the zone humidity ratio at the
\note humidistat set point so that dehumidification can occur.
\minimum> 0.0
\type real
\units kgWater/kgDryAir
N20, \field Zone Cooling Design Supply Air Humidity Ratio Difference
\note Zone Dehumidification Design Supply Air Humidity Ratio Difference is only used when
\note Zone Latent Cooling Design Supply Air Humidity Ratio Input Method = HumidityRatioDifference.
\note This input is a positive value and defines the difference between the zone humidity
\note ratio at the thermostat and humidistat set point condition and the supply air
\note humidity ratio entering the zone.
\minimum> 0.0
\type real
\default 0.005
\units kgWater/kgDryAir
A10, \field Zone Latent Heating Design Supply Air Humidity Ratio Input Method
\note Use SupplyAirHumidityRatio to enter the humidity ratio when zone humidification
\note is required. The supply air humidity ratio should be greater than the zone humidity
\note ratio at the zone thermostat and humidistat set point condition.
\note Use HumidityRatioDifference to enter the difference in humidity ratio from the
\note zone thermostat and humidistat set point condition.
\type choice
\key SupplyAirHumidityRatio
\key HumidityRatioDifference
\default HumidityRatioDifference
N21, \field Zone Humidification Design Supply Air Humidity Ratio
\note Zone Humidification Design Supply Air Humidity Ratio is only used when Zone Latent
\note Heating Design Supply Air Humidity Ratio Input Method = SupplyAirHumidityRatio.
\note This input must be greater than the zone humidity ratio at the
\note humidistat set point so that humidification can occur.
\minimum> 0.0
\type real
\units kgWater/kgDryAir
N22, \field Zone Humidification Design Supply Air Humidity Ratio Difference
\note Zone Humidification Design Supply Air Humidity Ratio is only used when Zone Latent
\note Heating Design Supply Air Humidity Ratio Input Method = HumidityRatioDifference.
\note This input is a positive value and defines the difference between the zone humidity
\note ratio at the thermostat and humidistat set point condition and the supply air
\note humidity ratio entering the zone.
\minimum 0.0
\type real
\default 0.005
\units kgWater/kgDryAir
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kbenne could you please take a look at this? These are new fields, likely not useful for most, so I left them with defaults and co instead of making them \required-field so they don't even show up in most cases when you serialize.

Could you advise whether you still would do them as required-field non optional, with default set in Ctor? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do personally prefer required-fields, but I see your point. I think the PR is fine the way it is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I feel bad and kinda want to make them required...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made them required in f5eccf0, except the ones without a default: E+ doesn't provide a default for Zone Dehumidification Design Supply Air Humidity Ratio (and Humidification counterpart) and I really don't have a good default either, so leaving this as optional

@jmarrec jmarrec requested a review from kbenne September 12, 2022 21:31
Conflicts:
	src/osversion/VersionTranslator.cpp
	src/osversion/test/VersionTranslator_GTest.cpp
E+ doesn't provide a default for `Zone Dehumidification Design Supply Air Humidity Ratio` and I really don't have a good default either, so leaving this as optional
@ci-commercialbuildings
Copy link
Collaborator

ci-commercialbuildings commented Sep 20, 2022

CI Results for ef4ad08:

@jmarrec jmarrec merged commit ea4bb1d into v22.2.0-IOFreeze Sep 20, 2022
@jmarrec jmarrec deleted the 4675_SizingZone_newFields branch September 20, 2022 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component - IDF Translation component - Model component - Version Translator Enhancement Request IDDChange Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E+ 22.2.0 - Sizing:Zone has new fields
4 participants