Merge pull request #1052 from NREL/check-buildstock2

Improve test/check_buildstock.rb

measures/ApplyUpgrade/measure.rb

@@ -270,7 +270,7 @@ def run(model, runner, user_arguments)
         register_value(runner, 'option_%02d_lifetime_to_apply' % option_num, lifetime.to_s)
 
         # Get measure name and arguments associated with the option
-        options_measure_args = get_measure_args_from_option_names(lookup_csv_data, [option_name], parameter_name, lookup_file, runner)
+        options_measure_args, _errors = get_measure_args_from_option_names(lookup_csv_data, [option_name], parameter_name, lookup_file, runner)
         options_measure_args[option_name].each do |measure_subdir, args_hash|
           system_upgrades = get_system_upgrades(hpxml, system_upgrades, args_hash)
           update_args_hash(measures, measure_subdir, args_hash, false)
@@ -289,7 +289,7 @@ def run(model, runner, user_arguments)
         parameters.each do |parameter_name|
           existing_option_name = values[OpenStudio::toUnderscoreCase(parameter_name)]
 
-          options_measure_args = get_measure_args_from_option_names(lookup_csv_data, [existing_option_name], parameter_name, lookup_file, runner)
+          options_measure_args, _errors = get_measure_args_from_option_names(lookup_csv_data, [existing_option_name], parameter_name, lookup_file, runner)
           options_measure_args[existing_option_name].each do |measure_subdir2, args_hash|
             next if measure_subdir != measure_subdir2
 

measures/ApplyUpgrade/measure.xml

@@ -3,8 +3,8 @@
   <schema_version>3.0</schema_version>
   <name>apply_upgrade</name>
   <uid>33f1654c-f734-43d1-b35d-9d2856e41b5a</uid>
-  <version_id>30a03e58-aaf7-4479-bbb1-9fda9283b2b7</version_id>
-  <version_modified>20230104T191300Z</version_modified>
+  <version_id>39d68fca-e8bb-4b12-97bd-414df1c3a3b1</version_id>
+  <version_modified>20230310T232749Z</version_modified>
   <xml_checksum>9339BE01</xml_checksum>
   <class_name>ApplyUpgrade</class_name>
   <display_name>Apply Upgrade</display_name>
@@ -6342,7 +6342,7 @@
       <filename>measure.rb</filename>
       <filetype>rb</filetype>
       <usage_type>script</usage_type>
-      <checksum>E8B729A9</checksum>
+      <checksum>CEBAEE16</checksum>
     </file>
   </files>
 </measure>

measures/BuildExistingModel/measure.rb

@@ -311,7 +311,7 @@ def run(model, runner, user_arguments)
     parameters_ordered.each do |parameter_name|
       option_name = bldg_data[parameter_name]
       print_option_assignment(parameter_name, option_name, runner)
-      options_measure_args = get_measure_args_from_option_names(lookup_csv_data, [option_name], parameter_name, lookup_file, runner)
+      options_measure_args, _errors = get_measure_args_from_option_names(lookup_csv_data, [option_name], parameter_name, lookup_file, runner)
       options_measure_args[option_name].each do |measure_subdir, args_hash|
         update_args_hash(measures, measure_subdir, args_hash, false)
       end

measures/BuildExistingModel/measure.xml

@@ -3,8 +3,8 @@
   <schema_version>3.0</schema_version>
   <name>build_existing_model</name>
   <uid>dedf59bb-3b88-4f16-8755-2c1ff5519cbf</uid>
-  <version_id>669b50df-3f02-4911-8032-eb629aa35902</version_id>
-  <version_modified>20230104T213808Z</version_modified>
+  <version_id>d24f574d-ebfc-43b9-8730-c179bd1b976f</version_id>
+  <version_modified>20230310T232750Z</version_modified>
   <xml_checksum>2C38F48B</xml_checksum>
   <class_name>BuildExistingModel</class_name>
   <display_name>Build Existing Model</display_name>
@@ -320,7 +320,7 @@
       <filename>measure.rb</filename>
       <filetype>rb</filetype>
       <usage_type>script</usage_type>
-      <checksum>F01D215C</checksum>
+      <checksum>22DEFC84</checksum>
     </file>
   </files>
 </measure>

measures/ServerDirectoryCleanup/measure.xml

@@ -3,8 +3,8 @@
   <schema_version>3.0</schema_version>
   <name>server_directory_cleanup</name>
   <uid>ec7d04ad-0b7b-495b-825a-e1b6d28d1d3f</uid>
-  <version_id>0eac2fa0-9256-4518-a588-4b75ccf9349b</version_id>
-  <version_modified>20230307T202937Z</version_modified>
+  <version_id>04aacca1-a5bc-4a91-9318-a28df1e492eb</version_id>
+  <version_modified>20230313T221238Z</version_modified>
   <xml_checksum>5F1EDF75</xml_checksum>
   <class_name>ServerDirectoryCleanup</class_name>
   <display_name>Server Directory Cleanup</display_name>

resources/buildstock.rb

@@ -302,6 +302,8 @@ def get_measure_args_from_option_names(lookup_csv_data, option_names, parameter_
     if (not row[0].nil?) && (not row[1].nil?)
       current_option = nil # reset
       option_names.each do |option_name|
+        next unless not option_name.nil?
+
         if (row[0].downcase == parameter_name.downcase) && (row[1].downcase == option_name.downcase)
           current_option = option_name
           break
@@ -327,12 +329,20 @@ def get_measure_args_from_option_names(lookup_csv_data, option_names, parameter_
       break if found_options.values.all? { |elem| elem == true }
     end
   end
+
+  errors = []
   option_names.each do |option_name|
-    if not found_options[option_name]
-      register_error("Could not find parameter '#{parameter_name}' and option '#{option_name}' in #{lookup_file}.", runner)
+    next unless not found_options[option_name]
+
+    msg = "Could not find parameter '#{parameter_name}' and option '#{option_name}' in #{lookup_file}."
+    if runner.nil?
+      errors << msg
+    else
+      register_error(msg, runner)
     end
   end
-  return options_measure_args
+
+  return options_measure_args, errors
 end
 
 def print_option_assignment(parameter_name, option_name, runner)

test/check_buildstock.rb

@@ -10,6 +10,7 @@
 options = {}
 OptionParser.new do |opts|
   opts.banner = "Usage: #{File.basename(__FILE__)} -o outfile -l lookup_file\n e.g., #{File.basename(__FILE__)} -o /path/to/buildstock.csv -l /path/to/options_lookup.tsv"
+  opts.banner += "\nThe appropriate options_lookup.tsv to check against is the one that will be used in the ResStock analysis."
 
   opts.on('-o', '--output <STRING>', 'Output file name') do |t|
     options[:outfile] = t
@@ -33,7 +34,10 @@
 lookup_file = options[:lookup_file]
 
 t0 = Time.now
-check_buildstock(outfile, lookup_file)
-t1 = Time.now
-
-puts "Checking took: #{((t1 - t0) / 60.0).round(1)} minutes."
+begin
+  check_buildstock(outfile, lookup_file)
+rescue Exception => e
+  puts e.message
+else
+  puts "Checking took: #{((Time.now - t0) / 60.0).round(1)} minutes."
+end

test/integrity_checks.rb

@@ -185,6 +185,7 @@ def integrity_check_options_lookup_tsv(project_dir_name, housing_characteristics
   model = OpenStudio::Model::Model.new
 
   # Gather all options/arguments
+  all_errors = []
   lookup_csv_data = CSV.open(lookup_file, col_sep: "\t").each.to_a
   parameter_names = get_parameters_ordered_from_options_lookup_tsv(lookup_csv_data)
   parameter_names.each do |parameter_name|
@@ -194,7 +195,8 @@ def integrity_check_options_lookup_tsv(project_dir_name, housing_characteristics
     next if not File.exist?(tsvpath) # Not every parameter used by every project
 
     option_names = get_options_for_parameter_from_options_lookup_tsv(lookup_csv_data, parameter_name)
-    options_measure_args = get_measure_args_from_option_names(lookup_csv_data, option_names, parameter_name, lookup_file, nil)
+    options_measure_args, errors = get_measure_args_from_option_names(lookup_csv_data, option_names, parameter_name, lookup_file)
+    all_errors += errors
     option_names.each do |option_name|
       check_for_illegal_chars(option_name, 'option')
 
@@ -224,6 +226,10 @@ def integrity_check_options_lookup_tsv(project_dir_name, housing_characteristics
     end
   end
 
+  if not all_errors.empty?
+    raise all_errors.map { |e| "ERROR: #{e}" }.join("\n")
+  end
+
   measures.keys.each do |measure_subdir|
     puts "Checking for issues with #{measure_subdir} measure..."
 
@@ -276,11 +282,16 @@ def integrity_check_options_lookup_tsv(project_dir_name, housing_characteristics
 def check_buildstock(output_file, lookup_file, lookup_csv_data = nil)
   require 'csv'
 
+  puts "Opening #{lookup_file}..."
   lookup_csv_data = CSV.open(lookup_file, col_sep: "\t").each.to_a if lookup_csv_data.nil?
 
   # Cache {parameter => options}
+  puts "Reading #{output_file}..."
   parameters_options = {}
-  CSV.foreach(output_file, headers: true).each do |row|
+  csv = CSV.foreach(output_file, headers: true)
+  raise 'ERROR: Missing the Building column.' if !csv.first.include?('Building')
+
+  csv.each do |row|
     row.each do |parameter_name, option_name|
       next if parameter_name == 'Building'
 
@@ -295,12 +306,20 @@ def check_buildstock(output_file, lookup_file, lookup_csv_data = nil)
   end
 
   # Cache {parameter => {option => {measure => {arg => value}}}}
+  puts 'Checking parameters/options...'
+  all_errors = []
   parameters_options_measure_args = {}
   parameters_options.each do |parameter_name, option_names|
-    parameters_options_measure_args[parameter_name] = get_measure_args_from_option_names(lookup_csv_data, option_names, parameter_name, lookup_file)
+    parameters_options_measure_args[parameter_name], errors = get_measure_args_from_option_names(lookup_csv_data, option_names, parameter_name, lookup_file)
+    all_errors += errors
+  end
+
+  if not all_errors.empty?
+    raise all_errors.map { |e| "ERROR: #{e}" }.join("\n")
   end
 
   # Check that measure arguments aren't getting overwritten
+  puts 'Checking for argument duplication...'
   err = ''
   CSV.foreach(output_file, headers: true).each do |row|
     args_map = {}

test/test_integrity_checks.rb

@@ -200,27 +200,31 @@ def test_buildstock_csv_valid
     assert(cli_output.include?('Checking took:'))
   end
 
-  def test_buildstock_csv_bad_parameter
+  def test_buildstock_csv_bad_parameters
     begin
       outfile = File.join(File.dirname(__FILE__), '..', 'test/tests_buildstock_csvs/buildstock_csv_bad_parameter/buildstock.csv')
       lookup_file = File.join(File.dirname(__FILE__), '..', 'resources', 'test_options_lookup.tsv')
       check_buildstock(outfile, lookup_file)
     rescue Exception => e
       puts e.message
       assert(e.message.include? "ERROR: Could not find parameter 'Location2' and option 'AL_Birmingham.Muni.AP.722280' in")
+      assert(e.message.include? "ERROR: Could not find parameter 'Location3' and option 'AL_Birmingham.Muni.AP.722280' in")
+      assert(e.message.include? "ERROR: Could not find parameter 'Location4' and option 'AL_Birmingham.Muni.AP.722280' in")
     else
       flunk "Should have caused an error but didn't."
     end
   end
 
-  def test_buildstock_csv_bad_option
+  def test_buildstock_csv_bad_options
     begin
       outfile = File.join(File.dirname(__FILE__), '..', 'test/tests_buildstock_csvs/buildstock_csv_bad_option/buildstock.csv')
       lookup_file = File.join(File.dirname(__FILE__), '..', 'resources', 'test_options_lookup.tsv')
       check_buildstock(outfile, lookup_file)
     rescue Exception => e
       puts e.message
       assert(e.message.include? "ERROR: Could not find parameter 'Vintage' and option '<1940s' in")
+      assert(e.message.include? "ERROR: Could not find parameter 'Vintage' and option '1940ss' in")
+      assert(e.message.include? "ERROR: Could not find parameter 'Vintage' and option '<1950s' in")
     else
       flunk "Should have caused an error but didn't."
     end

test/tests_buildstock_csvs/buildstock_csv_bad_option/buildstock.csv

@@ -1,4 +1,4 @@
 Building,Location,Vintage
 1,AL_Birmingham.Muni.AP.722280,<1940s
-2,AL_Huntsville.Intl.AP-Jones.Field.723230,1940s
-3,AL_Mobile-Rgnl.AP.722230,<1950
+2,AL_Huntsville.Intl.AP-Jones.Field.723230,1940ss
+3,AL_Mobile-Rgnl.AP.722230,<1950s

test/tests_buildstock_csvs/buildstock_csv_bad_parameter/buildstock.csv

@@ -1,4 +1,4 @@
-Building,Location2,Vintage
-1,AL_Birmingham.Muni.AP.722280,<1940
-2,AL_Huntsville.Intl.AP-Jones.Field.723230,1940s
-3,AL_Mobile-Rgnl.AP.722230,<1950
+Building,Location2,Location3,Location4,Vintage
+1,AL_Birmingham.Muni.AP.722280,AL_Birmingham.Muni.AP.722280,AL_Birmingham.Muni.AP.722280,<1940
+2,AL_Huntsville.Intl.AP-Jones.Field.723230,AL_Huntsville.Intl.AP-Jones.Field.723230,AL_Huntsville.Intl.AP-Jones.Field.723230,1940s
+3,AL_Mobile-Rgnl.AP.722230,AL_Mobile-Rgnl.AP.722230,AL_Mobile-Rgnl.AP.722230,<1950