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

Prepare for 3.8.0 release by running a 3-way comparison with 3.8.0-rc2 and adjusting tests #207

Merged
merged 11 commits into from
May 7, 2024

Conversation

jmarrec
Copy link
Contributor

@jmarrec jmarrec commented May 7, 2024

Pull request overview

No diffs due to OpenStudio, small differences (but lots of them) due to bumping E+. Max diff from bumping E+ is 0.8% so small indeed

image

@jmarrec jmarrec added the Other PR type: Something else, like maintenance of the repo, or just committing test results label May 7, 2024
Comment on lines +65 to +69
hx = coil_system.heatExchanger.to_HeatExchangerAirToAirSensibleAndLatent.get
hx.setName('CoilSystemWaterHX HX')
if Gem::Version.new(OpenStudio.openStudioVersion) >= Gem::Version.new('3.8.0')
hx.assignHistoricalEffectivenessCurves
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Backward compat for HX AirToAir Sensible and Latent.

OS aligned the default on E+, which means constant efficiency (and no need for Table:Lookup objects). But to maintain historical EUI, need to use the convenience function to set the efficiency at 75% like it was pre 24.1.0


windows = []
spaces.each do |space|
surfaces = space.surfaces.sort_by { |s| s.name.to_s }
surfaces = space.surfaces.sort_by(&:azimuth)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The surfaces are not in a consistent naming convention, so instead we sort by azimuth, which ensures we always take for eg the East wall, and not randomly the West, North, South one

Comment on lines +1484 to +1489
if std.respond_to?('construction_calculated_solar_heat_gain_coefficient')
shgc = std.construction_calculated_solar_heat_gain_coefficient(construction_root)
else
# 0.6.0+
shgc = OpenstudioStandards::Constructions.construction_get_solar_transmittance(surface_construction)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started porting some of the code in openstudio_results to use newer methods and avoid sections breaking because NoMethodError or Optional not initialized, but there would be more to do to get it perfect.

@DavidGoldwasser as you already know, even if this specific measure inside this repo is very outdated, the most current on openstudio-common-measures-gem will also need some of the same treatment

Comment on lines +424 to +429
if standard.respond_to?('model_add_ground_temperatures')
standard.model_add_ground_temperatures(model, primary_bldg_type, climate_zone)
else
# 0.6.0+, model_set_ground_temperatures
OpenstudioStandards::Weather.model_set_ground_temperatures(model, climate_zone: climate_zone)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same stuff, that was causing an actual crash in model_articulation1 test. Because unlike openstudio_results, which just skips sections/table when something fails (a begin/rescue block is used), this crashes.

@jmarrec jmarrec merged commit 751c126 into develop May 7, 2024
4 checks passed
@jmarrec jmarrec deleted the 3.8.0-rc2 branch May 7, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Other PR type: Something else, like maintenance of the repo, or just committing test results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant