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

Refactor Weather module #1650

Merged
merged 59 commits into from
Feb 7, 2024
Merged

Refactor Weather module #1650

merged 59 commits into from
Feb 7, 2024

Conversation

eringold
Copy link
Collaborator

@eringold eringold commented Jan 2, 2024

Pull request overview

Adds weather-related information and modify methods, as well as classes for parsing EPW and STAT files. Includes all functionality of ChangeBuildingLocation measure to standards.

Pull Request Author

  • Method changes or additions
  • Data changes or additions
  • Added tests for added methods
  • If methods have been deprecated, update rest of code to use the new methods
  • Documented new methods using yard syntax
  • Resolved yard documentation errors for new code (ran bundle exec rake doc)
  • Resolved rubocop syntax errors for new code (ran bundle exec rake rubocop)
  • All new and existing tests passes

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a code review on GitHub
  • All related changes have been implemented: method additions, changes, tests
  • Check rubocop errors
  • Check yard doc errors
  • If fixing a defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • If a new feature, test the new feature and try creative ways to break it
  • CI status: all green or justified

eringold and others added 12 commits January 2, 2024 09:47
- add monthly_lagged_dry_bulb field to stat_file to replace model_get_monthly_ground_temps_from_stat_file
- model_standards_climate_zone -> model_get_climate_zone
- removed get_climate_zone_code; `climate_zone.split('-')[-1]` returns the same thing
- moved model_get_full_weather_file_path
- moved model_set_climate_zone
- rubocop edits
Copy link
Collaborator

@mdahlhausen mdahlhausen left a comment

Choose a reason for hiding this comment

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

@eringold changes look good. I moved over more methods and deleted the old ones. I'm going to deal with Weather.Model.rb and the WeatherFile class in this PR before I merge it.

eringold and others added 9 commits January 23, 2024 16:25
- replace calculate_humidity_ratio in Weather.Model.rb with epw_file_get_dehumidification_degree_days
- remove Weather.Model.rb and references to it
Add the Calgary Intl AP Ann Clg .4% Condns WB=>MDB to the expected_result.osm for the regression tests.
The design day was seemingly intended to be included in Weather.Model.rb, but it never made it into the expected results for some reason.
update performance results to account for an extra design day in the models meaning more energy use
@mdahlhausen
Copy link
Collaborator

@srgilani Could you review the changes from these two commits?:

I can't quite recreate the values in Graham Wright's paper for average global irradiance. The values I do calculate are reasonable and accurate, but are bit higher than his range. The dehumidification degree days are roughly the same - perhaps a bit higher. I noticed your method would take the average for an entire day, and set ddds to zero if the average did not meet the 0.010 threshold. My method sums all hourly values over 0.010, then divides by 24 hours - similar to how one would calculate heating or cooling degree days.

Copy link
Collaborator

@srgilani srgilani left a comment

Choose a reason for hiding this comment

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

@mdahlhausen I reviewed your methods and went through ASHRAE Chapter 14 that you used for calculating global irradiance; agreed with your methods/changes. I had only a comment/question about your changes in btap_data.rb

average_daily_global_irradiance_w_per_m2 = OpenstudioStandards::Weather.design_day_average_global_irradiance(design_day)
average_daily_global_irradiance_w_per_m2_array << average_daily_global_irradiance_w_per_m2
end
solar_irradiance_on_heating_design_day_w_per_m_sq = average_daily_global_irradiance_w_per_m2_array.min
Copy link
Collaborator

Choose a reason for hiding this comment

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

I did not follow why min (or max) of "average_daily_global_irradiance_w_per_m2_array" is calculated here... wouldn't that be only one day which is WinterDesignDay (or SummerDesignDay)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So what this is saying is for the heating condition - look at all the heating design days, and log the average daily global irradiance.
Then, take the minimum of these values, meaning use the one which has the lowest average daily global irradiance. This is the most extreme condition - heating with the least amount of solar gain.

The inverse is for cooling - we want the most extreme condition, which is cooling with the most amount of solar gain.

# puts "For local_standard_time_hour #{local_standard_time_hour}, apparent_solar_time #{apparent_solar_time}, hour_angle_degrees #{hour_angle_degrees}, solar_altitude_degrees #{solar_altitude_degrees}, air_mass #{air_mass}, beam_normal_irradiance #{beam_normal_irradiance}, diffuse_horizontal_irradiance #{diffuse_horizontal_irradiance}"
end

average_daily_global_irradiance = global_irradiance_array.sum / 24.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed you are averaging over 24 hours of the day. I am still not sure what PHIUS meant by that…

Copy link
Collaborator

@mdahlhausen mdahlhausen Jan 26, 2024

Choose a reason for hiding this comment

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

Yes, I went with the daily average. It's both a bit more straightforward to calculate, and gives numbers closer to the range in Graham Wright's paper. "Only during sun hours" is both more complicated, and means that the average value across design days isn't comparable, because the sun hours aren't the same.


# calculate dehumidification degree days based on humidity ratio values above the base
hr_values_above_base = hr_values.map{ |hr| hr > base_humidity_ratio ? hr : 0.0}
dehumidification_degree_days = hr_values_above_base.sum / 24.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried to find any sources for it; the only thing I found is that Graham Wright says in his paper that “DDD was calculated by thresholding the monthly humidity data at a humidity ratio of 0.010”. To me it sounded like they considered monthly average, so I used it as daily humidity ratio…
I tested for Montreal, the two methods give 0.76 using your method and 0.15 using my method… I’m not sure which one would make more sense for what PHIUS is asking for… we can use the same method as heating/cooling degree days as you suggested

Copy link
Collaborator

@mdahlhausen mdahlhausen Jan 26, 2024

Choose a reason for hiding this comment

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

If Graham is using monthly humidity averages, then both of our methods are wrong. It also wouldn't be "degree-days" in the normal meaning of the term. In the standard calculation of heating degree days for example, it's often the case that the sum of monthly heating degree days at 18C are non-zero when the monthly average temperature is above 18C.
I think the best way to resolve this is to write up and implement a proposal to PHIUS to implement a better and clearer regression. But I don't have time for that. Until then I suggest we settle with the methods we have that at least are clear and somewhat defensible.

@ckirney
Copy link
Collaborator

ckirney commented Jan 26, 2024

I will make a branch and test with our costing stuff to make sure there are no issues.

several NECB regression tests have minute changes in equipment sizes in fields and object names
Copy link
Collaborator

@ckirney ckirney left a comment

Choose a reason for hiding this comment

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

I noticed some small changes in results but it looks good to me.

@mdahlhausen
Copy link
Collaborator

@ckirney thanks for the review! The changes are from adding the annual wetbulb condition. It seems that was intended to be added to the necb models, but the specific regex used wasn't adding it.

@mdahlhausen mdahlhausen merged commit 7f71c2a into master Feb 7, 2024
@mdahlhausen mdahlhausen deleted the refactor/weather_module branch February 7, 2024 21:37
@mdahlhausen mdahlhausen changed the title Refactor weather methods Refactor Weather module Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants