Handle incomplete EPW design conditions header

[joseph-robertson]

Pull request overview

• Fixes EPWDesignCondition getters should return boost::optional #5132.
• EpwDesignCondition has many API-breaking changes related to its getters. The previous behavior was to misleadingly return a value of 0 for any empty design condition header field. The types for the getters are now either boost::optional<double> or boost::optional<int>.

Notes:

• Does stringToDouble return 0.0 if an empty string is passed in? Yes.
• Should the following changes be made to EpwFile.hpp / EpwFile.cpp:
  • check for empty strings in setters; e.g., check heatingDryBulb99 is empty string in setHeatingDryBulb99? Yes.
  • change all getters to return boost::optional<double>; e..g, double EpwDesignCondition::heatingDryBulb99() const { to boost::optional<double> EpwDesignCondition::heatingDryBulb99() const {? Yes.
  • change, e.g., double m_heatingDryBulb99 to boost::optional<double> m_heatingDryBulb99 in EpwDesignCondition class? Yes.

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]

Rebased onto develop and resolved conflicts.

[jmarrec]

Made a couple of minor changes

[jmarrec]

I didn't find occurrences of this being used anywhere in the common openstudio-gems I have on my machine right now.

The only one I can find is ResStock's weather.rb, but I'm sure @joseph-robertson is well aware he'll need to change that so I'm fine with the API Break. @kbenne, grand protector of the API, could you chime in for confirmation?

https://github.com/NREL/resstock/blob/280a58c2686f30a1b09f10ba3640e49c265d06f3/resources/hpxml-measures/HPXMLtoOpenStudio/resources/weather.rb#L261-L276)

[joseph-robertson]

I didn't find occurrences of this being used anywhere in the common openstudio-gems I have on my machine right now.

The only one I can find is ResStock's weather.rb, but I'm sure @joseph-robertson is well aware he'll need to change that so I'm fine with the API Break. @kbenne, grand protector of the API, could you chime in for confirmation?

https://github.com/NREL/resstock/blob/280a58c2686f30a1b09f10ba3640e49c265d06f3/resources/hpxml-measures/HPXMLtoOpenStudio/resources/weather.rb#L261-L276)

@jmarrec I think OS-HPXML's weather.rb was originally what motivated this PR; there's no way to know if it's actually zero vs. missing.

However, did you see my "-9999" question here? #5318 (comment)

[jmarrec]

It's debatable whether -9999 or boost::optional is better than the other, and I'd be fine with both probably. As long as we convey that it's missing, that's probably fine.

The -9999 option does allow not breaking API, which is a plus. And even for dry bulb temperature, you could always check if < -100C and you'd be guaranted to know it's missing.
The downside is that it's not as clear that it's missing (dereferencing an empty optional will produce a crash, while you could technically just that -9999 value for calculations and just get bogus results without noticing). But as a mitigation factor: 1) the EPW format itself includes some -99 or -9999 values to indicate missing values (and so does EnergyPlus internally for AutoSized fields for eg), and 2) I don't think this is an area of the SDK that is largely used.

[joseph-robertson]

It's debatable whether -9999 or boost::optional is better than the other, and I'd be fine with both probably. As long as we convey that it's missing, that's probably fine.

The -9999 option does allow not breaking API, which is a plus. And even for dry bulb temperature, you could always check if < -100C and you'd be guaranted to know it's missing. The downside is that it's not as clear that it's missing (dereferencing an empty optional will produce a crash, while you could technically just that -9999 value for calculations and just get bogus results without noticing). But as a mitigation factor: 1) the EPW format itself includes some -99 or -9999 values to indicate missing values (and so does EnergyPlus internally for AutoSized fields for eg), and 2) I don't think this is an area of the SDK that is largely used.

@jmarrec Hm. Well what's your vote here?

Should we at least be consistent with the new EPW ground temperatures implementation? I.e., change all ground temps getters to boost::optionals if we decide to go with this PR as-is?

[ci-commercialbuildings]

CI Results for 7e68d39:

• Ubuntu 24.04:

  • OpenStudio-3.9.1-alpha+7e68d39ba1-Ubuntu-24.04-x86_64.deb
  • OpenStudio-3.9.1-alpha+7e68d39ba1-Ubuntu-24.04-x86_64.tar.gz

• Ubuntu 22.04:

  • OpenStudio-3.9.1-alpha+7e68d39ba1-Ubuntu-22.04-x86_64.deb
  • OpenStudio-3.9.1-alpha+7e68d39ba1-Ubuntu-22.04-x86_64.tar.gz
  • perf_test/results.html

• Darwin:

• Windows:

  • OpenStudio-3.9.1-alpha+7e68d39ba1-Windows.exe
  • OpenStudio-3.9.1-alpha+7e68d39ba1-Windows.tar.gz

• Code Coverage:

[jmarrec]

Let's go with this PR's optional implementation.
I made the ground soil properties be an optional, as these are missing usually, kept the rest as double.