-
Notifications
You must be signed in to change notification settings - Fork 190
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
Addresses #5214, EpwFile getTimeSeries fails for leap year weather file w/no leap day #5217
base: develop
Are you sure you want to change the base?
Conversation
After a bit of digging, it appears that Therefore we fall into this block instead of this one. If |
Ah ha, here's the core issue. For all 3 of the above scenarios,
This is the line that fails. The assumedYear in the Data ctor here is 2009, and so fails on the line above. Why doesn't the EpwDataPoint have a dateTime of Answer:
|
@@ -3967,6 +3974,31 @@ bool EpwFile::parse(std::istream& ifs, bool storeData) { | |||
} | |||
} | |||
|
|||
for (unsigned int i = 0; i < epw_strings.size(); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do this after the previous loop, instead of inside it, because we first need to get realYear
.
CI Results for 17d344f:
|
The fix looks good. I tried it on Windows with a couple of different weather files that had the issue before and everything seems to work. Thanks for resolving this. |
Pull request overview
As far as I can tell, the
Bad Date: year = 2009, month = Feb(2), day = 29.
error is thrown for each of the following scenarios:Possible solutions:
isActualOverride
bool supplied toEpwFile::getTimeSeries
(demonstrated in this PR)m_data
to change xxxx-Feb-29 00:00:00 to xxxx-Mar-01 00:00:00 for TMY with leap FebPull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.