-
Notifications
You must be signed in to change notification settings - Fork 394
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 uninit water mains #8677
Fix uninit water mains #8677
Conversation
That's an interesting error. I'm going to get a redo on Windows before investigating. |
It builds fine on my Windows VM FWIW |
Windows built fine on CI, this is good to go as far as CI goes. This also doesn't cause any diffs, which is a bit surprising if it's actually being used uninitialized. Perhaps it's only being used uninitialized at the very beginning? Before reporting happens? In any case, it looks like this hasn't affected any actual results, so I'm feeling OK with this. I'll take one more look at the change but I think this will go in shortly. |
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.
Yeah, again, I think this must've only been using it uninitialized early in the program before outputs were generated because this hasn't caused any strange behavior or spurious diffs that I know of. I'm good with this quick fix.
this->SnowGndRefModifierForDayltg = 1.0; // Modifier to ground reflectance during snow for daylighting | ||
this->SnowGndRefModifier = 1.0; // Modifier to ground reflectance during snow | ||
this->SnowGndRefModifierForDayltg = 1.0; // Modifier to ground reflectance during snow for daylighting | ||
this->WaterMainsTempsMethod = WeatherManager::WaterMainsTempCalcMethod::FixedDefault; |
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.
Got it, this is during the clear_state call, and then...
RptIsRain(0), RptIsSnow(0), RptDayType(0), HrAngle(0.0), SolarAltitudeAngle(0.0), SolarAzimuthAngle(0.0), HorizIRSky(0.0), | ||
TimeStepFraction(0.0), NumSPSiteScheduleNamePtrs(0), EndDayOfMonth(12, {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}), LeapYearAdd(0), | ||
DatesShouldBeReset(false), StartDatesCycleShouldBeReset(false), Jan1DatesShouldBeReset(false), RPReadAllWeatherData(false) | ||
SnowGndRefModifierForDayltg(1.0), WaterMainsTempsMethod{WeatherManager::WaterMainsTempCalcMethod::FixedDefault}, WaterMainsTempsSchedule(0), |
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.
This was during the default constructor of the data class (which I don't think we actually need...but that's a different story).
Thanks @lefticus , merging. |
This fixes use of uninitialized WaterMainsTempMethod