diff --git a/docs/scenario_workflow_translation_plan.md b/docs/scenario_workflow_translation_plan.md index cbbf54e..7aa13de 100644 --- a/docs/scenario_workflow_translation_plan.md +++ b/docs/scenario_workflow_translation_plan.md @@ -99,8 +99,9 @@ lib/BOSS/scenario_measure_map.json Lookup order: -1. `SystemCategoryAffected` plus `MeasureName`. -2. Technology category element plus `MeasureName`, when `SystemCategoryAffected` is missing or not useful. +1. Technology category element plus `MeasureName`. +2. `SystemCategoryAffected` plus `MeasureName`, when the technology category is blank, matches no mapping, or matches mappings with different steps. +3. Otherwise the measure is unmapped and reported as a warning. BOSS does not map by `MeasureName` alone or guess between mappings with different steps. The legacy `../BuildingSync-gem` mapping can be used as background, but do not copy it directly. @@ -195,13 +196,13 @@ Use this roadmap as the tracker. Keep `Status` and `Delivered by` current when a - Done when: Unit tests return expected steps and structured unmapped warnings. - Avoid: Mutating reader data, raising on unmapped measures, or hardcoding mapping rules outside JSON. -### PR 7: Technology Category Fallback +### PR 7: Technology Category Lookup -- Status: Planned -- Delivered by: - -- Scope: Add fallback lookup by technology category plus `MeasureName`. -- Done when: Tests prove fallback works when `SystemCategoryAffected` is missing and normal lookup priority still wins. -- Avoid: Changing normal category/name lookup behavior. +- Status: Done +- Delivered by: `scenario-workflow-7-technology-category-fallback` branch +- Scope: Look up mappings by technology category plus `MeasureName` first, then fall back to `SystemCategoryAffected` plus `MeasureName`. +- Done when: Tests prove technology category lookup wins, `SystemCategoryAffected` fallback works when the technology category is blank, unmatched, or ambiguous, and unresolved measures warn instead of guessing. +- Avoid: Changing mapped results for `building_151.xml` measures, mapping by `MeasureName` alone, or guessing between mappings with different steps. ### PR 8: Conditional Mapping Rules @@ -367,9 +368,24 @@ Use this section for stable facts future PRs need. Keep entries short and tied t - Verification: `C:\Ruby32-x64\bin\ruby.exe -S bundle exec rspec spec/tests/unit/scenario_measure_mapper_spec.rb` - 11 examples, 0 failures. - Verification: `C:\Ruby32-x64\bin\ruby.exe -S bundle exec rspec spec/tests/unit` - 78 examples, 0 failures, no constant redefinition warnings. - Verification: `bundle exec rubocop` on the changed files reports only `Layout/EndOfLine` on Windows working copies; git normalizes line endings on commit. -- Follow-up: PR 7 should handle blank `system_category_affected` by falling back to `technology_category_element_name`, which narrows `:measure_lookup_key_incomplete` to blank `measure_name` only. +- Follow-up: PR 7 should handle blank `system_category_affected` by falling back to `technology_category_element_name`, which narrows `:measure_lookup_key_incomplete` to blank `measure_name` only. Superseded: PR 7 made technology category the primary lookup instead. - Follow-up for PR 12/13: `OpenStudio::Extension.set_measure_argument` matches every step with the same `measure_dir_name` unless `step_name` is passed. `SetLightingLoadsByLPD` is both a baseline step and the LED mapping target. Append mapped steps with their arguments already set, or pass a unique `step_name`, so baseline step arguments are not overwritten. +### PR 7: Technology Category Lookup + +- Status: Done +- Delivered by: `scenario-workflow-7-technology-category-fallback` branch +- Handoff: `#map_measure` looks up `[technology_category_element_name, measure_name]` first. If that key is blank, has no JSON entry, or matches entries with different steps, it looks up `[system_category_affected, measure_name]`. Otherwise the measure is unmapped. Both keys use the PR 6 normalization, and BOSS never maps by `measure_name` alone. +- Handoff: Technology category matches with identical steps collapse to one mapping. If the technology category match is ambiguous and `SystemCategoryAffected` does not resolve it, the result is `steps: []` with `:ambiguous_technology_category_mapping`. Other misses return `:unmapped_measure`. A future window-specific air sealing entry would share `BuildingEnvelopeModifications` + `Air seal envelope` with the wall entry, so `SystemCategoryAffected` would choose between them. +- Handoff: `:measure_lookup_key_incomplete` means blank `measure_name`, or both `technology_category_element_name` and `system_category_affected` blank. Mapper warnings and log lines also include `technology_category_element_name`. +- Handoff: `scenario_measure_map.json` `lookup_order` is `technology_category_element_name_and_measure_name`, then `system_category_affected_and_measure_name`. It documents the order; the mapper does not read it. Every entry must have a non-empty `technology_category_element_name`, and the 12 current values were checked against `../schema/BuildingSync.xsd` and `building_151.xml` reader output. `building_151.xml` still maps 12 of 16 measures with the same steps as PR 6. +- Handoff: Fixture outcomes: `179D_Example_Building.xml` has no `SystemCategoryAffected`; technology category lookup maps its air sealing and LED measures, and its two `ServiceHotWaterSystems` measures stay unmapped. `Fenestration` + `Air seal envelope` in `Example_ASHRAE_L2_Report_-_with_Energy_Use_Data.xml` is a window air sealing upgrade and maps through `BuildingEnvelopeModifications` to the whole-building `ReduceSpaceInfiltrationByPercentage` step. `Upgrade operating protocols, calibration, and/or sequencing` in `Example_HOMES_Template_Building.xml` with `Other HVAC`/`BuildingAutomationSystems` or `Domestic Hot Water`/`WaterAndSewerConservationSystems` matches neither key, so both stay `:unmapped_measure`. +- Verification: `C:\Ruby32-x64\bin\ruby.exe -S bundle exec rspec spec/tests/unit/scenario_measure_mapper_spec.rb spec/tests/unit/scenario_measure_map_spec.rb` - 21 examples, 0 failures. +- Verification: `C:\Ruby32-x64\bin\ruby.exe -S bundle exec rspec spec/tests/unit` - 83 examples, 0 failures. +- Verification: `bundle exec rubocop` on the changed Ruby files reports only `Layout/EndOfLine` on the Windows working copy of `scenario_measure_mapper_spec.rb`. +- Follow-up: PR 8 conditional rules must keep the technology-category-first lookup. Entries that share a technology category and `MeasureName` with different steps are allowed, but each needs a distinct `SystemCategoryAffected` so the fallback can choose one. +- Follow-up: PR 17 already has lookup priority, fallback, and ambiguity coverage from PR 7; extend it only for gaps PR 8 introduces. + ## Verification Commands Use the narrowest command that proves the current PR. Broaden only when the change justifies it. diff --git a/lib/BOSS/scenario_measure_map.json b/lib/BOSS/scenario_measure_map.json index dbcedfb..1b78c4e 100644 --- a/lib/BOSS/scenario_measure_map.json +++ b/lib/BOSS/scenario_measure_map.json @@ -1,6 +1,7 @@ { "schema_version": "1.0", "lookup_order": [ + "technology_category_element_name_and_measure_name", "system_category_affected_and_measure_name" ], "mappings": [ diff --git a/lib/BOSS/scenario_measure_mapper.rb b/lib/BOSS/scenario_measure_mapper.rb index bc394e0..e2161ff 100644 --- a/lib/BOSS/scenario_measure_mapper.rb +++ b/lib/BOSS/scenario_measure_mapper.rb @@ -13,28 +13,45 @@ module BOSS # Maps one parsed BuildingSync measure (a value from BuildingSyncReader#get_measures) # to OpenStudio OSW step specs using scenario_measure_map.json. + # + # Lookup tries the technology category element name plus MeasureName first. When that is + # blank, unmatched, or matches mappings with different steps, it tries SystemCategoryAffected + # plus MeasureName. Otherwise the measure is unmapped. class ScenarioMeasureMapper def initialize(map_path = SCENARIO_MEASURE_MAP_PATH) @map_path = map_path - @mappings_by_key = _index_mappings(_load_mappings(map_path)) + mappings = _load_mappings(map_path) + @mappings_by_technology_category_key = _index_mappings(mappings, :technology_category_element_name) + @mappings_by_system_category_key = _index_mappings(mappings, :system_category_affected) end # @param measure [Hash] symbol-keyed measure hash from the reader # @return [Hash] { measure_id:, steps: [ { measure_dir_name:, arguments: {} } ], warnings: [] } def map_measure(measure) - system_category_affected = measure[:system_category_affected] - measure_name = measure[:measure_name] - key = _lookup_key(system_category_affected, measure_name) + technology_category_key = _lookup_key(measure[:technology_category_element_name], measure[:measure_name]) + system_category_key = _lookup_key(measure[:system_category_affected], measure[:measure_name]) - if key.nil? + if technology_category_key.nil? && system_category_key.nil? return _unmapped_result( measure, :measure_lookup_key_incomplete, - 'Measure is missing SystemCategoryAffected or MeasureName needed for mapping lookup.' + 'Measure needs MeasureName plus a technology category or SystemCategoryAffected for mapping lookup.' + ) + end + + technology_category_mappings = @mappings_by_technology_category_key.fetch(technology_category_key, []) + mapping = technology_category_mappings.first if technology_category_mappings.length == 1 + mapping ||= @mappings_by_system_category_key.fetch(system_category_key, []).first + + if mapping.nil? && technology_category_mappings.length > 1 + return _unmapped_result( + measure, + :ambiguous_technology_category_mapping, + 'Measure matches multiple OpenStudio mappings by technology category and MeasureName, ' \ + 'and SystemCategoryAffected does not choose one.' ) end - mapping = @mappings_by_key[key] if mapping.nil? return _unmapped_result(measure, :unmapped_measure, 'Measure has no OpenStudio mapping.') end @@ -63,24 +80,27 @@ def _load_mappings(map_path) return mappings end - def _index_mappings(mappings) + # Groups mappings by [category, MeasureName] in JSON order. Entries with identical steps + # collapse to one, so only mappings with different steps make a key ambiguous. + def _index_mappings(mappings, category_field) mappings_by_key = {} mappings.each do |mapping| - key = _lookup_key(mapping[:system_category_affected], mapping[:measure_name]) - next if key.nil? || mappings_by_key.key?(key) + key = _lookup_key(mapping[category_field], mapping[:measure_name]) + next if key.nil? - mappings_by_key[key] = mapping + key_mappings = (mappings_by_key[key] ||= []) + key_mappings << mapping if key_mappings.none? { |key_mapping| key_mapping[:steps] == mapping[:steps] } end return mappings_by_key.freeze end - def _lookup_key(system_category_affected, measure_name) - system_category_key = _normalize(system_category_affected) + def _lookup_key(category, measure_name) + category_key = _normalize(category) measure_name_key = _normalize(measure_name) - return nil if system_category_key.nil? || measure_name_key.nil? + return nil if category_key.nil? || measure_name_key.nil? - return [system_category_key, measure_name_key] + return [category_key, measure_name_key] end def _normalize(value) @@ -95,12 +115,14 @@ def _unmapped_result(measure, code, message) message:, measure_id: measure[:measure_id], system_category_affected: measure[:system_category_affected], + technology_category_element_name: measure[:technology_category_element_name], measure_name: measure[:measure_name] } BOSS.BOSS_logger.warn( "BOSS.ScenarioMeasureMapper: #{message} " \ "(measure_id: #{warning[:measure_id].inspect}, " \ "system_category_affected: #{warning[:system_category_affected].inspect}, " \ + "technology_category_element_name: #{warning[:technology_category_element_name].inspect}, " \ "measure_name: #{warning[:measure_name].inspect})" ) diff --git a/spec/tests/unit/scenario_measure_map_spec.rb b/spec/tests/unit/scenario_measure_map_spec.rb index 8413d5f..c395252 100644 --- a/spec/tests/unit/scenario_measure_map_spec.rb +++ b/spec/tests/unit/scenario_measure_map_spec.rb @@ -9,10 +9,15 @@ it 'defines required mapping fields for each entry' do expect(scenario_measure_map.fetch('schema_version')).to eq '1.0' + expect(scenario_measure_map.fetch('lookup_order')).to eq [ + 'technology_category_element_name_and_measure_name', + 'system_category_affected_and_measure_name' + ] expect(mappings).not_to be_empty mappings.each do |mapping| expect(mapping.fetch('system_category_affected')).not_to be_empty + expect(mapping.fetch('technology_category_element_name')).not_to be_empty expect(mapping.fetch('measure_name')).not_to be_empty expect(mapping.fetch('steps')).not_to be_empty diff --git a/spec/tests/unit/scenario_measure_mapper_spec.rb b/spec/tests/unit/scenario_measure_mapper_spec.rb index b26c57d..d1610a0 100644 --- a/spec/tests/unit/scenario_measure_mapper_spec.rb +++ b/spec/tests/unit/scenario_measure_mapper_spec.rb @@ -17,12 +17,25 @@ def mapper_fixture_path(*parts) File.expand_path(File.join('../../files', *parts), __dir__) end - def building_151_measures - xml_path = mapper_fixture_path('v2.7.0', 'building_151.xml') + def fixture_measures(file_name) + xml_path = mapper_fixture_path('v2.7.0', file_name) doc = REXML::Document.new(File.read(xml_path), ignore_whitespace_nodes: :all) BOSS::BuildingSyncReader.new(doc, nil, ASHRAE90_1).get_measures end + def building_151_measures + fixture_measures('building_151.xml') + end + + def single_step_mapping(system_category_affected, technology_category_element_name, measure_name, measure_dir_name) + { + system_category_affected:, + technology_category_element_name:, + measure_name:, + steps: [{ measure_dir_name:, arguments: { __SKIP__: false } }] + } + end + def with_map_file(content) file = Tempfile.new(['scenario_measure_map', '.json']) file.write(content) @@ -98,6 +111,7 @@ def lighting_measure(overrides = {}) message: 'Measure has no OpenStudio mapping.', measure_id: 'Measure18', system_category_affected: 'Heating System', + technology_category_element_name: 'BoilerPlantImprovements', measure_name: 'Replace boiler' ) end @@ -117,23 +131,152 @@ def lighting_measure(overrides = {}) expect(result[:warnings]).to eq [] end - it 'warn without raising when MeasureName is missing' do + it 'warn without raising when MeasureName or both categories are missing' do + # Set Up + incomplete_measures = [ + lighting_measure(measure_name: nil), + lighting_measure(system_category_affected: nil, measure_name: ' '), + lighting_measure(system_category_affected: ' ', technology_category_element_name: nil) + ] + + incomplete_measures.each do |measure| + # Action + result = mapper.map_measure(measure) + + # Assert + expect(result[:steps]).to eq [] + expect(result[:warnings].map { |warning| warning[:code] }).to eq [:measure_lookup_key_incomplete] + expect(result[:warnings].first).to include(measure.merge(severity: :warning)) + end + end + + it 'map by technology category plus MeasureName when SystemCategoryAffected is missing' do + # Set Up + expected_steps = mapper.map_measure(lighting_measure)[:steps] + technology_category_measures = [nil, '', ' '].map { |blank_category| lighting_measure(system_category_affected: blank_category) } + technology_category_measures << lighting_measure( + system_category_affected: nil, + technology_category_element_name: ' lightingimprovements ', + measure_name: 'Retrofit with light emitting diode technologies' + ) + + technology_category_measures.each do |measure| + # Action and Assert + expect(mapper.map_measure(measure)).to eq(measure_id: 'Measure1', steps: expected_steps, warnings: []) + end + end + + it 'map building_151 measures the same way by either category lookup' do + building_151_measures.each_value do |measure| + # Action + full_result = mapper.map_measure(measure) + category_only_results = [ + mapper.map_measure(measure.merge(system_category_affected: nil)), + mapper.map_measure(measure.merge(technology_category_element_name: nil)), + mapper.map_measure(measure.merge(technology_category_element_name: 'UnmappedTechnology')) + ] + + # Assert + category_only_results.each do |result| + if full_result[:steps].any? + expect(result).to eq full_result + else + expect(result[:warnings].map { |warning| warning[:code] }).to eq [:unmapped_measure] + end + end + end + end + + it 'map 179D_Example_Building measures that have no SystemCategoryAffected' do + # Set Up + measures = fixture_measures('179D_Example_Building.xml') + expect(measures.values.map { |measure| measure[:system_category_affected] }).to all(be_nil) + # Action - result = mapper.map_measure(lighting_measure(measure_name: nil)) + results = measures.transform_values { |measure| mapper.map_measure(measure) } # Assert - expect(result[:steps]).to eq [] - expect(result[:warnings].map { |warning| warning[:code] }).to eq [:measure_lookup_key_incomplete] - expect(result[:warnings].first).to include(measure_id: 'Measure1', severity: :warning, measure_name: nil) + expect(results.fetch('MeasureType-45099320')[:steps]).to eq [ + { measure_dir_name: 'ReduceSpaceInfiltrationByPercentage', arguments: { __SKIP__: false, space_infiltration_reduction_percent: 30.0 } } + ] + expect(results.fetch('MeasureType-45099340')[:steps]).to eq [ + { measure_dir_name: 'SetLightingLoadsByLPD', arguments: { __SKIP__: false, lpd: 0.6 } } + ] + ['MeasureType-45099360', 'MeasureType-45099380'].each do |measure_id| + expect(results.fetch(measure_id)[:steps]).to eq [] + expect(results.fetch(measure_id)[:warnings].first).to include(code: :unmapped_measure, technology_category_element_name: 'ServiceHotWaterSystems') + end end - it 'warn without raising when SystemCategoryAffected is missing' do + it 'prefer technology category lookup over SystemCategoryAffected lookup' do + # Set Up + map_json = JSON.generate( + mappings: [ + single_step_mapping('Lighting', 'OtherTechnology', 'Shared measure', 'SystemCategoryMatch'), + single_step_mapping('Other System', 'LightingImprovements', 'Shared measure', 'TechnologyCategoryMatch') + ] + ) + + with_map_file(map_json) do |map_path| + # Action + result = BOSS::ScenarioMeasureMapper.new(map_path).map_measure(lighting_measure(measure_name: 'Shared measure')) + + # Assert + expect(result[:steps].map { |step| step[:measure_dir_name] }).to eq ['TechnologyCategoryMatch'] + expect(result[:warnings]).to eq [] + end + end + + it 'map Fenestration air sealing by technology category and leave cross-category protocol measures unmapped' do + # Set Up + fenestration_measure = fixture_measures('Example_ASHRAE_L2_Report_-_with_Energy_Use_Data.xml').fetch('MeasureType-45035260') + expect(fenestration_measure).to include(system_category_affected: 'Fenestration', technology_category_element_name: 'BuildingEnvelopeModifications') + homes_measures = fixture_measures('Example_HOMES_Template_Building.xml') + protocol_measures = homes_measures.values_at('MeasureType-45139460', 'MeasureType-45139560') + expect(protocol_measures.map { |measure| measure[:system_category_affected] }).to eq ['Other HVAC', 'Domestic Hot Water'] + expect(protocol_measures.map { |measure| measure[:technology_category_element_name] }).to eq ['BuildingAutomationSystems', 'WaterAndSewerConservationSystems'] + # Action - result = mapper.map_measure(lighting_measure(system_category_affected: ' ')) + fenestration_result = mapper.map_measure(fenestration_measure) + protocol_results = protocol_measures.map { |measure| mapper.map_measure(measure) } # Assert - expect(result[:steps]).to eq [] - expect(result[:warnings].map { |warning| warning[:code] }).to eq [:measure_lookup_key_incomplete] + expect(fenestration_result[:steps]).to eq mapper.map_measure(building_151_measures.fetch('Measure3'))[:steps] + expect(fenestration_result[:warnings]).to eq [] + protocol_results.each do |result| + expect(result[:steps]).to eq [] + expect(result[:warnings].map { |warning| warning[:code] }).to eq [:unmapped_measure] + end + end + + it 'fall back to SystemCategoryAffected when the technology category lookup is ambiguous' do + # Set Up + map_json = JSON.generate( + mappings: [ + single_step_mapping('Wall', 'BuildingEnvelopeModifications', 'Air seal envelope', 'WallAirSeal'), + single_step_mapping('Roof', 'BuildingEnvelopeModifications', 'Air seal envelope', 'RoofAirSeal'), + single_step_mapping('Wall', 'BuildingEnvelopeModifications', 'Shared steps', 'SameStep'), + single_step_mapping('Roof', 'BuildingEnvelopeModifications', 'Shared steps', 'SameStep') + ] + ) + envelope_measure = { measure_id: 'Measure3', technology_category_element_name: 'BuildingEnvelopeModifications', measure_name: 'Air seal envelope' } + + with_map_file(map_json) do |map_path| + map_path_mapper = BOSS::ScenarioMeasureMapper.new(map_path) + + # Action + ambiguous_results = [nil, 'Fenestration'].map { |category| map_path_mapper.map_measure(envelope_measure.merge(system_category_affected: category)) } + same_steps_result = map_path_mapper.map_measure(envelope_measure.merge(measure_name: 'Shared steps')) + categorized_result = map_path_mapper.map_measure(envelope_measure.merge(system_category_affected: 'Roof')) + + # Assert + ambiguous_results.each do |result| + expect(result[:steps]).to eq [] + expect(result[:warnings].map { |warning| warning[:code] }).to eq [:ambiguous_technology_category_mapping] + end + expect(same_steps_result).to include(steps: [{ measure_dir_name: 'SameStep', arguments: { __SKIP__: false } }], warnings: []) + expect(categorized_result[:steps].map { |step| step[:measure_dir_name] }).to eq ['RoofAirSeal'] + end end it 'log a warning for unmapped measures' do