Appendix g dev by weilixu · Pull Request #1612 · NatLabRockies/openstudio-standards

weilixu · 2023-09-22T04:40:42Z

Pull request overview

Merge Appendix G PRM development.

Features:

Wx/baseline oa #1514 Add baseline outdoor air workflow
Proposed model #1556 Add the capability to generate a proposed building model from a user model - this includes adjust infiltration values and lighting power density for dwelling units and model/hotel, equipment schedule for computer space.
Appx g/feature/export user data partial #1611 Add export user data capability that can export a user data template based on user model.

Enhancements:

Wx bug fix fenestration expansion missing construction name #1507 Handle missing standard space type in user model
Wx/baseline oa #1514 Add OSSTD-PRM exception handler module to handle 1. get OpenStudio option objects, 2. handle reading the user data from csv file, 3. handle getting missing additional properties.
Complete run four orientations #1533 Update the function run_four_orientation using methods in exception handler module.
Appx g/ehancement/update ground temperature profile #1549 Update the function update_ground_temperature_profile using methods in exception handler module.
reorganize the function #1550 Update the function model_identify_non_mech_cooled_systems using methods in exception handler module.
add exterior lighting test function #1552 Update the function apply_baseline_exterior_lighting using methods in exception handler module, move user data to user data process function
reorganize the elevator function and enhance the exceptional handling #1553 Update the function model_add_prm_elevators using methods in exception handler module.
Appx g/enhancement/restructure user data #1608 Restructure user data module to create a enum library, a validation workflow and a data processing workflow.

Bug fixs:

Adding assert to handle model generating unsuccessfully case in the r… #1405 Unit test: handling unsuccessful proposed model generation
Appendix g dev debug user directory error #1407 Fix reading user data directory error
Appendix g dev debug unmet load hour issues #1428 Resolve infinite loop in unmet load hour check and debug SQL session block in Windows OS.
Appx g/bugfix/duplicate ltg schedules #1588 Address the issue described in 90.1 PRM: Redundant SpaceType Lights schedule created for each Space in SpaceType #1527 - spaceType lights schedule duplicated.
Include fenestration types_to_modify to apply correct PRM constructio… #1589 Address fenestration missing constructions when apply PRM method
Appx g/merge master 09192023 #1610 Fix generating water heating coil in PSZ-VAV system causing EnergyPlus crash for duplicated components error.

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
If the code adds new require statements, ensure these are in core ruby or add to the gemspec

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

…s not assigned, which cause the incorrect baseline construction assignment, and ultimately energyplus simulation failure.

…missing_construction_name Wx bug fix fenestration expansion missing construction name

…andards into wx/baseline_oa

…cation Add assertions for getting data from additional properties Add new file import in openstudio-standards.rb Add new process in the master function under Standards.Model.rb

…very case.

Wx/baseline oa

update the prm_raise

…ations Complete run four orientations

…baseline_window_to_wall_ratio

…_baseline_window_to_wall_ratio Appx g/enhancement/model apply prm baseline window to wall ratio

lib/openstudio-standards/standards/Standards.PlantLoop.rb

mdahlhausen · 2023-09-22T05:22:43Z

lib/openstudio-standards/utilities/assertion.rb

+# @param data_key [string] The data key to retrieve the data from the OpenStudio object
+# @param remaining_keys [str] Any additional keys in the path
+# @return the OpenStudio Object or exception raise
+def prm_get_optional_handler(component, log_dir, data_key, *remaining_keys)


Why is this function necessary?

We have many situations to get optional object from an OpenStudio object.

spacetype = space.spaceType.get.standardsSpaceType.get

In above code, if a space has no spaceType or standardsSpaceType, then ruby would raise exception with message nil has no get function. This happens quite often with models generated from BEM software. The error message is not informative.

The prm_get_optional_handler is designed to handle such scenario to give more informative error messages to user.

The better practice is to call .is_initialized each time, and then have comments at that point in code if the optional value is not initialized.

Search .is_initialized in the codebase and you'll see many examples of if - else statements to catch this sort of thing.

Right, but there are chained optionals in the PRM codebase, so this method comes in handy to handle those chained optional get.
I can add .is_initialized in the prm_get_optional_handler method to reflect the best practice?

I see 20 uses of this method in the code base, and 1 instance of chaining, which I elsewhere commented wasn't necessary.
I think this method doesn't accomplish what you want - giving more clear error messages to the user.

In the PRM routine, there are growing number of chaining for processing user data including some of them pointing to name.
Examples include:
Get building user data name
get space types:

Most of them are specific to PRM user data processing so I would like, if possible, to keep it for now. is_initialized can handle if a specific optional data exist or not, but the detail implementation to handle this condition varies. This method is convenient to handle is_initialized = false condition to fail the process with information on which optional object is missing from a component.

I can update the error message to include an OpenStudio.logFree(OpenStudio::Error, 'openstudio.standards.utilities', 'key xxx is missing from xxx object...) before line 47 so we get a user message saying something is missing, instead of nil has no get function?

mdahlhausen · 2023-09-22T05:23:46Z

lib/openstudio-standards/standards/ashrae_90_1_prm/userdata_csv/userdata_enums.rb

why the use of global variables instead of instance variables for class UserData?

I thought about using global variables, but it loses some functionalities specific to a group of data.

For example, there are many flags in the user data that require user to type true or false in the CSV file.
To check whether user provided correct string to these flags, we need to have a way to verify those strings are not yes or no and they are case insensitive.

The advantage of class implementation is that the UserData class can have a generic matched_any? method which compares a string with a list of expected enums.
In the case described above, we created a UserDataBoolean, a subclass of UserData, and specified expected enums (true and false) as instance variables to run the matched_any?.

lib/openstudio-standards/standards/ashrae_90_1_prm/ashrae_90_1_prm.Surface.rb

lib/openstudio-standards/standards/ashrae_90_1_prm/ashrae_90_1_prm.SpaceType.rb

mdahlhausen · 2023-09-22T05:32:59Z

lib/openstudio-standards/standards/Standards.PlantLoop.rb

+      end
+
+      # Get pipe node
+      node = prm_get_optional_handler(pipe, @sizing_run_dir, 'to_StraightComponent', 'outletModelObject', 'to_Node')


why not use the outletModelObject field from the StraightComponent parent class instead?

That line is equivalent to pipe.to_StraightComponent.outletModelObject.to_Node, so we do use the outletModelObject field from the StraightComponent parent class. Could you please clarify what you would like to see?

just replace it with node = pipe.to_StraightComponent.outletModelObject.to_Node

also not sure node_name is used in the following lines?

Doesn't to_Node return an optional? prm_get_optional_handler handles optionals. I can remove the node name.

lib/openstudio-standards/utilities/assertion.rb

mdahlhausen · 2023-09-22T05:37:54Z

test/90_1_prm/test_appendix_g_prm.rb

I should have mentioned this last time:
Split out this file into a helper methods file, and a test file. The test_appendix_g_prm should only include the test_ methods. (It sometimes makes sense to break this practice, but I don't it makes sense here).

Make senses. Given the time constraint, could we move this to next release? I will open an issue in our backlog to address it.

it's simple:

move non-test methods to another file

add a require statement at the top of the test file

Done, removed.
It could be further improved, I will put those work to our blacklog. Thanks @mdahlhausen

mdahlhausen · 2023-09-22T05:41:58Z

lib/openstudio-standards/utilities/object_info.rb

+  #
+  # @param [OpenStudio::Model::ModelObject]
+  # @return Casted OpenStudio object or nil if the cast was not possible
+  def model_cast_model_object(model_object)


I think this method is unnecessary and could result in weird behavior, particularly for HVAC where it is called. I suggest refactoring to remove it in the two places it is used. Happy to think about how to refactor those methods.

This method is actually quite convenient. For example, with a few lines of code, it allows us to check if an HVAC related component in a model is autosized without actually knowing what its full object type name is in advance. Without it, we would have to list in the code all the objects that we want to check. The proposed approach also supports future objects to be included in OpenStudio/EnergyPlus (i.e., new coil objects). It has existed in the codebase for a while now, we just generalized it here to use it in multiple places.

What are the weird behavior that you'd expect to see? If you don't want to expose it we could make it private.

lib/openstudio-standards/utilities/object_info.rb

mdahlhausen · 2023-09-22T05:43:33Z

lib/openstudio-standards/utilities/object_info.rb

Can probably delete this file as methods are unused or could be refactored and embedded inline.

lib/openstudio-standards/utilities/assertion.rb

mdahlhausen · 2023-09-22T17:35:20Z

lib/openstudio-standards/standards/Standards.Model.rb

@@ -1,4 +1,5 @@
 require 'csv'
+require 'date'


is this in the core or std library?

I think this comes from Ruby standard library - see Ruby 2.7.2

lib/openstudio-standards/standards/ashrae_90_1_prm/ashrae_90_1_prm.Model.rb

mdahlhausen · 2023-09-22T17:58:17Z

lib/openstudio-standards/standards/ashrae_90_1_prm/ashrae_90_1_prm.Model.rb

@@ -2182,6 +2272,7 @@ def model_get_bat_wwr_target(bat, wwr_list)
  # @param total_fene_m2 [Float] total fenestration area
  # @return [Float] reduction factor
  def model_get_wwr_reduction_ratio(multiplier,


method should be surface_get_wwr_reduction_ratio.
And then just pass in the surface. You pass in name, and results from methods with surface as an argument.

Do you suggest passing in a surface and then get surface_name, surface_wwr and surface_dr shall be a local variable inside the model_get_wwr_reduction_ratio?

This function has updated to surface_get_wwr_reduction_ratio with surface as a new argument to replace surface_name, surface_wwr and surface_dr

…_helper 2. Modify the function model_get_wwr_reduction_ratio to surface_get_wwr_reduction and reduce its argument to surface. - Update YARD document

weilixu added 30 commits May 2, 2023 22:12

research

3b12969

Fix bug -> when expanding the / adding new window, the construction i…

c9b18f3

…s not assigned, which cause the incorrect baseline construction assignment, and ultimately energyplus simulation failure.

add warning and handle missing defaultConstructionSet with exception.

78ec223

Add assertion function

0fe207d

update the text,

f59af57

Update the logic based on comments

d162f5f

add function to handle user data inputs for OA

0c13fb4

run CI

c85f6a1

add read user data function to read user data with default value assign

2373bd2

add message for TODO

06716c0

Merge pull request #1507 from NREL/wx_bug_fix_fenestration_expansion_…

ba7ab64

…missing_construction_name Wx bug fix fenestration expansion missing construction name

Merge branch 'AppendixG_Dev' of https://github.com/NREL/openstudio-st…

7b9a475

…andards into wx/baseline_oa

Update adding the function to check user data for outdoor air specifi…

503a216

…cation Add assertions for getting data from additional properties Add new file import in openstudio-standards.rb Add new process in the master function under Standards.Model.rb

add handeler to handle the get situation - will likely to do it for e…

402670d

…very case.

update optional handler to simpify the call stack

156ca94

add log for data conversion

33b729e

no message

dadcb62

add test case

e42b68a

Fix error in the assertion function

fcdf723

update the prm_raise

bc32ec1

Merge pull request #1514 from NREL/wx/baseline_oa

51dd654

Wx/baseline oa

Merge pull request #1532 from NREL/wx/baseline_oa

8a1ecff

update the prm_raise

Complete run four orientations

d141592

Merge pull request #1533 from NREL/appx_g/enhancement/run_four_orient…

62249d8

…ations Complete run four orientations

add new assertion function

d97effc

added logs

e552542

update the output log message

31f8819

update with logs and error message

b2e45fa

Merge branch 'AppendixG_Dev' into appx_g/enhancement/model_apply_prm_…

16a2277

…baseline_window_to_wall_ratio

Merge pull request #1536 from NREL/appx_g/enhancement/model_apply_prm…

acaa6c9

…_baseline_window_to_wall_ratio Appx g/enhancement/model apply prm baseline window to wall ratio