Fix #5518 - Design conditions incorrectly parsed for TMYx EPWs #5521
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have compared the chapter 14 of the ASHRAE Handbook of Fundamentals for all vintages from 2009 to 2025: https://docs.google.com/spreadsheets/d/14wPHNDVmDZyLqcGrGDeW8pH6tx_psQn8-8SZ1Fp6tJY/edit?usp=sharing
…e section to the Cooling one, reuse the same field
jmarrec
commented
Nov 7, 2025
Collaborator
Author
jmarrec
left a comment
There was a problem hiding this comment.
TL;DR:
- Parse the title string of the DESIGN CONDITIONS to try to determine ASHRAE HoF Vintage
- If present and valid (2009 and increments of four years), check the expected number of fields in each Heating and Extremes section: if not matching, we warn and fall back to using heuristics
- If no parsed versions or invalid, we use heuristics: we look at the number of found fields in the Heating and Extremes sections to determine the HoF vintage that matches
- Final check to ensure that everything is ok, including the number of cooling fields (which do not change)
- Then we assign the heating, cooling, and extremes sections separately, handling the fields based on vintage.
Comment on lines
+132
to
+139
| // cppcheck-suppress [unknownMacro, syntaxError] | ||
| OPENSTUDIO_ENUM(ASHRAEHoFVintage, | ||
| ((ASHRAE_2009)(ASHRAE Handbook of Fundamentals 2009)) | ||
| ((ASHRAE_2013)(ASHRAE Handbook of Fundamentals 2013)) | ||
| ((ASHRAE_2017)(ASHRAE Handbook of Fundamentals 2017)) | ||
| ((ASHRAE_2021)(ASHRAE Handbook of Fundamentals 2021)) | ||
| ((ASHRAE_2025)(ASHRAE Handbook of Fundamentals 2025)) | ||
| ); |
Collaborator
Author
There was a problem hiding this comment.
New enum for ASHRAE Handbook of Fundamentals vintage
| const size_t num_strings = strings.size(); | ||
| // Expect 68 items in the strings | ||
| if (strings.size() < 68) { | ||
| if (num_strings < 67 || num_strings > 68) { |
Collaborator
Author
There was a problem hiding this comment.
Bail early. I have never seen a TMYx file prepared with HoF 2017 vintage climatic design conditions which has one field less than the rest, but I suppose it could happen.
Comment on lines
+685
to
+712
| const auto& title = strings[EpwDesignField::TitleOfDesignCondition]; | ||
| static const boost::regex yearRegex(R"(\b(20\d{2})\b)"); // Matches 20xx | ||
| boost::optional<ASHRAEHoFVintage> assumedVersion; | ||
| boost::smatch m; | ||
| if (boost::regex_search(title, m, yearRegex)) { | ||
| int year = std::stoi(m[1]); | ||
| if (year >= 2009 && (year - 2009) % 4 == 0) { | ||
| // Valid ASHRAE design year: 2009, 2013, 2017, 2021, 2025, etc. | ||
| if (year == 2009) { | ||
| assumedVersion = ASHRAEHoFVintage::ASHRAE_2009; | ||
| } else if (year == 2013) { | ||
| assumedVersion = ASHRAEHoFVintage::ASHRAE_2013; | ||
| } else if (year == 2017) { | ||
| assumedVersion = ASHRAEHoFVintage::ASHRAE_2017; | ||
| } else if (year == 2021) { | ||
| assumedVersion = ASHRAEHoFVintage::ASHRAE_2021; | ||
| } else if (year == 2025) { | ||
| assumedVersion = ASHRAEHoFVintage::ASHRAE_2025; | ||
| } else { | ||
| // Assume future version is the same as 2025 | ||
| LOG_FREE(Debug, "openstudio.EpwFile", | ||
| "Unrecognized ASHRAE design year (" << year << ") in EPW design condition title. Assuming 2025 vintage for parsing."); | ||
| assumedVersion = ASHRAEHoFVintage::ASHRAE_2025; | ||
| } | ||
| } else { | ||
| LOG_FREE(Debug, "openstudio.EpwFile", "Did not detect a valid ASHRAE design year in EPW design condition title."); | ||
| } | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
So, I start by trying to figure out the version in the title. Technically speaking, this isn't necessary, but I would like to Warn the user if there is a valid year source in there but the number of fields don't match: we look for 20xx and if it's 2009 or any four (4) year increment, it's valid.
Comment on lines
+714
to
+741
| auto itHeating = std::find_if(strings.cbegin(), strings.cend(), [](const auto& s) { return openstudio::istringEqual(s, "Heating"); }); | ||
| if (itHeating == strings.cend()) { | ||
| LOG_FREE(Error, "openstudio.EpwFile", "Could not find 'Heating' design condition in EPW design condition string"); | ||
| return boost::none; | ||
| } | ||
| // Heating Must be in position 2 (Title, Blank, Heating, ...) | ||
| if (std::distance(strings.cbegin(), itHeating) != 2) { | ||
| LOG_FREE(Error, "openstudio.EpwFile", "'Heating' design condition not in expected position in EPW design condition string"); | ||
| return boost::none; | ||
| } | ||
| auto itCooling = std::find_if(strings.cbegin(), strings.cend(), [](const auto& s) { return openstudio::istringEqual(s, "Cooling"); }); | ||
| if (itCooling == strings.cend()) { | ||
| LOG_FREE(Error, "openstudio.EpwFile", "Could not find 'Cooling' design condition in EPW design condition string"); | ||
| return boost::none; | ||
| } | ||
| auto itExtreme = std::find_if(strings.cbegin(), strings.cend(), [](const auto& s) { return openstudio::istringEqual(s, "Extremes"); }); | ||
| if (itExtreme == strings.cend()) { | ||
| LOG_FREE(Error, "openstudio.EpwFile", "Could not find 'Extremes' design condition in EPW design condition string"); | ||
| return boost::none; | ||
| } | ||
|
|
||
| std::span<const std::string> heatingStrings(std::next(itHeating, 1), itCooling); | ||
| std::span<const std::string> coolingStrings(std::next(itCooling, 1), itExtreme); | ||
| std::span<const std::string> extremesStrings(std::next(itExtreme, 1), strings.cend()); | ||
|
|
||
| const auto numHeatingFields = heatingStrings.size(); | ||
| const auto numCoolingFields = coolingStrings.size(); | ||
| const auto numExtremeFields = extremesStrings.size(); |
Collaborator
Author
There was a problem hiding this comment.
Now we chunk into std::spans by looking for the "Heating", "Cooling", and "Extremes" fields.
If we can't find them, bail.
Comment on lines
+744
to
+783
| // constexpr size_t numOtherFields = 5; // Title, Blank, Heating, Cooling, Extreme | ||
| size_t expectedNumHeatingFields = 15; | ||
| const size_t expectedNumCoolingFields = 32; | ||
| size_t expectedNumExtremeFields = 16; | ||
|
|
||
| bool needHeuristics = false; | ||
|
|
||
| if (assumedVersion) { | ||
| if (*assumedVersion <= ASHRAEHoFVintage::ASHRAE_2017) { | ||
| } else { | ||
| expectedNumHeatingFields = 16; | ||
| } | ||
| if (*assumedVersion <= ASHRAEHoFVintage::ASHRAE_2013) { | ||
| expectedNumHeatingFields = 15; | ||
| expectedNumExtremeFields = 16; | ||
| } else if (*assumedVersion <= ASHRAEHoFVintage::ASHRAE_2017) { | ||
| expectedNumExtremeFields = 15; | ||
| expectedNumExtremeFields = 15; | ||
| } else { | ||
| expectedNumExtremeFields = 16; | ||
| expectedNumExtremeFields = 15; | ||
| } | ||
| // Check that the field counts match the assumed version, we'll fall back to heuristics if not | ||
| if (numHeatingFields != expectedNumHeatingFields) { | ||
| LOG_FREE(Warn, "openstudio.EpwFile", | ||
| "Based on the assumed vintage parsed in the title string of " << assumedVersion->valueDescription() << ", we expected " | ||
| << expectedNumHeatingFields << " Heating fields but got " | ||
| << numHeatingFields << ", falling to back to heuristics."); | ||
| needHeuristics = true; | ||
| } | ||
| if (numExtremeFields != expectedNumExtremeFields) { | ||
| LOG_FREE(Warn, "openstudio.EpwFile", | ||
| "Based on the assumed vintage parsed in the title string of " << assumedVersion->valueDescription() << ", we expected " | ||
| << expectedNumExtremeFields << " Extreme fields but got " | ||
| << numExtremeFields << ", falling to back to heuristics."); | ||
| needHeuristics = true; | ||
| } | ||
| } else { | ||
| needHeuristics = true; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
If there was a parsable version in the title, we check that the number of fields are matching what we expect. If not, we warn and still fall back to using heuristics.
Comment on lines
+596
to
+598
| if (m_ashraeHoFVersion >= ASHRAEHoFVintage::ASHRAE_2021) { | ||
| setHeatingWindShelterFactor(stringValues[index(EpwDesignField::HeatingWindShelterFactor)]); | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
assignHeatingData: we parse Wind Shelter Factor if >= 2021
Comment on lines
+637
to
+641
| if (m_ashraeHoFVersion <= ASHRAEHoFVintage::ASHRAE_2013) { | ||
| setCoolingHours8To4AndDryBulb12pt8To20pt6(stringValues[index(EpwDesignField::CoolingHours8To4AndDryBulb12pt8To20pt6)]); | ||
| } else { | ||
| setExtremeMaxWetBulb(stringValues[index(EpwDesignField::CoolingExtremeMaxWetBulb) - 1]); | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
assignCoolingData: depending on vintage, the last field is CoolingHours8To4AndDryBulb12pt8To20pt6 or CoolingExtremeMaxWetBulb (notice the index -1 here)
Comment on lines
+644
to
+657
| void EpwDesignCondition::assignExtremesData(std::span<const std::string> stringValues) { | ||
| size_t offset = 1; | ||
| auto index = [&offset, base_field = static_cast<size_t>(EpwDesignField::Extremes)](EpwDesignField field) -> size_t { | ||
| return static_cast<size_t>(field.value()) - base_field - offset; | ||
| }; | ||
|
|
||
| setExtremeWindSpeed1(stringValues[index(EpwDesignField::ExtremeWindSpeed1)]); | ||
| setExtremeWindSpeed2pt5(stringValues[index(EpwDesignField::ExtremeWindSpeed2pt5)]); | ||
| setExtremeWindSpeed5(stringValues[index(EpwDesignField::ExtremeWindSpeed5)]); | ||
| if (m_ashraeHoFVersion <= ASHRAEHoFVintage::ASHRAE_2013) { | ||
| setExtremeMaxWetBulb(stringValues[index(EpwDesignField::ExtremeMaxWetBulb)]); | ||
| } else { | ||
| ++offset; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
assignExtremesData: skip or setExtremeMaxWetBulb depending on vintage
Comment on lines
+1124
to
+1131
| TEST(Filetypes, EpwDesignCondition_HoF_2009) { | ||
|
|
||
| const std::string line = "Climate Design Data 2009 ASHRAE Handbook,," | ||
| "Heating,12,-18.8,-15.5,-21.6,0.7,-10.9,-18.8,0.9,-7.5,12.2,3.9,10.9,3.8,3,340," | ||
| "Cooling,7,15.2,33,15.7,32,15.5,30.2,15.3,18.4,27.3,17.5,26.4,16.8,25.6,4.9,0,16.1,14.3,20.2,14.9,13.2,19.9,13.9,12.3," | ||
| "19.6,59.7,27.3,56.6,26.6,54,25.7,760," | ||
| "Extremes,11.1,9.5,8.4,22.9,-22.9,36.1,3.8,1.2,-25.7,37,-27.9,37.7,-30.1,38.3,-32.8,39.2"; | ||
| auto dc_ = EpwDesignCondition::fromDesignConditionsString(line); |
Collaborator
Author
There was a problem hiding this comment.
Lots of tests, will not comment on them
Comment on lines
+1496
to
+1507
| TEST(Filetypes, EpwDesignCondition_HoF_2021_Heuristics_OverrideVersion) { | ||
|
|
||
| const std::string line = "This title includes the 2009 version when it's not actually it,," | ||
| "Heating,12,-18.8,-15.5,-21.6,0.7,-10.9,-18.8,0.9,-7.5,12.2,3.9,10.9,3.8,3,340,0.635," | ||
| "Cooling,7,15.2,33,15.7,32,15.5,30.2,15.3,18.4,27.3,17.5,26.4,16.8,25.6,4.9,0,16.1,14.3,20.2,14.9,13.2,19.9,13.9,12.3," | ||
| "19.6,59.7,27.3,56.6,26.6,54,25.7,22.9," | ||
| "Extremes,11.1,9.5,8.4,-22.9,36.1,3.8,1.2,-25.7,37,-27.9,37.7,-30.1,38.3,-32.8,39.2"; | ||
| auto dc_ = EpwDesignCondition::fromDesignConditionsString(line); | ||
| ASSERT_TRUE(dc_); | ||
| const auto dc = std::move(*dc_); | ||
|
|
||
| EXPECT_EQ(dc.ashraeHoFVersion(), ASHRAEHoFVintage::ASHRAE_2021); |
Collaborator
Author
There was a problem hiding this comment.
[ RUN ] Filetypes.EpwDesignCondition_HoF_2021_Heuristics_OverrideVersion
[openstudio.EpwFile] <0> Based on the assumed vintage parsed in the title string of ASHRAE Handbook of Fundamentals 2009, we expected 15 Heating fields but got 16, falling to back to heuristics.
[openstudio.EpwFile] <0> Based on the assumed vintage parsed in the title string of ASHRAE Handbook of Fundamentals 2009, we expected 16 Extreme fields but got 15, falling to back to heuristics.
Collaborator
|
CI Results for c5a4e41:
|
Collaborator
Author
|
I'm going to take the silence as an approbation, I want to start producing nightly builds for testing, so merging. We can address any comment that may arise later (I assume they'll be quite minor, I'm pretty confident in the changes here). Merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request overview
The summary of the differences is this:
Pull Request Author
src/model/test)src/energyplus/Test)src/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.