diff --git a/docs/scenario_workflow_translation_plan.md b/docs/scenario_workflow_translation_plan.md index 7aa13de..359ca22 100644 --- a/docs/scenario_workflow_translation_plan.md +++ b/docs/scenario_workflow_translation_plan.md @@ -206,9 +206,9 @@ Use this roadmap as the tracker. Keep `Status` and `Delivered by` current when a ### PR 8: Conditional Mapping Rules -- Status: Planned -- Delivered by: - -- Scope: Add data-driven conditional arguments for building type and principal HVAC/system context. +- Status: In Progress +- Delivered by: `scenario-workflow-8-conditional-mapping-rules` branch +- Scope: Add data-driven conditional arguments for building type and principal HVAC/system context. Mapper engine and tests only; the PR 5 deferred production mappings are separate follow-up work. - Done when: Tests prove conditions include and exclude arguments predictably. - Avoid: Per-measure Ruby condition chains. @@ -351,7 +351,7 @@ Use this section for stable facts future PRs need. Keep entries short and tied t - Delivered by: PR #10, merged into `scenario-workflow` via PR #5 - Handoff: `lib/BOSS/scenario_measure_map.json` maps 12 `building_151.xml` category/name pairs to OSW-shaped steps with `measure_dir_name` and an argument hash that includes `__SKIP__`. Source strings match reader output exactly. - Handoff: Roof and wall absolute insulation targets use `buildingsync-measures-gem` `modify_envelope_insulation` with SI R-values (R-30 IP = 5.28, R-25 IP = 4.40 m²K/W). Other mappings use `openstudio-ee` or `openstudio-common-measures`. Relative mappings such as air sealing, ceiling insulation by percentage, and thermal bypasses stay on relative measures. -- Deferred: Daylight controls, package unit replacement, burner replacement, and boiler replacement. The `buildingsync-measures-gem` split HVAC measures (`modify_existing_hvac_equipment_efficiencies`, `modify_existing_plant_equipment`, `hvac_packaged_rooftop_*`) need exact model object or zone names. Map them in PR 8 with context rules after `buildingsync-measures-gem` PR #3 merges. +- Deferred: Daylight controls, package unit replacement, burner replacement, and boiler replacement. The `buildingsync-measures-gem` split HVAC measures (`modify_existing_hvac_equipment_efficiencies`, `modify_existing_plant_equipment`, `hvac_packaged_rooftop_*`) need exact model object or zone names. Map them in a follow-up after `buildingsync-measures-gem` PR #3 merges and exact target names can be supplied; PR 8 only adds the conditional-argument engine they may use. - Verification: JSON parse passed; every mapped measure directory and argument was checked against installed `BOSS.gemspec` gems and `buildingsync-measures-gem` `develop`. - Verification: `C:\Ruby32-x64\bin\ruby.exe -S bundle exec rspec spec/tests/unit/scenario_measure_map_spec.rb`. - Follow-up: PR 6 adds `BOSS::ScenarioMeasureMapper`. PR 12 must put `buildingsync-measures-gem` on OSW `measure_paths`. @@ -386,6 +386,20 @@ Use this section for stable facts future PRs need. Keep entries short and tied t - 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. +### PR 8: Conditional Mapping Rules + +- Status: In Progress +- Delivered by: `scenario-workflow-8-conditional-mapping-rules` branch +- Handoff: `#map_measure(measure, context: {})` accepts `building_type` (first value of `BuildingSyncReader#get_building_type_and_bar_division_method`) and `hvac_system_type` (`#get_principal_HVAC_system_type`, the mapped OpenStudio name). The caller builds the context; the mapper does not read XML. `nil` context is empty; other non-Hash values raise `ArgumentError`. Extra keys are ignored. Return shape and warnings are unchanged. +- Handoff: A step may have optional `conditional_arguments`, an ordered array of `{ "when": {...}, "arguments": {...} }`. `when` keys are `building_type` and `hvac_system_type`; each value is a non-blank string or non-empty string array. All keys must match (AND); any array value may match (OR); matching uses the lookup normalization. Missing, blank, or non-string context never matches and is not a warning. +- Handoff: After PR 7 lookup selects a mapping, each step is deep-copied, `conditional_arguments` is removed, and every matching rule's arguments merge into the step arguments in JSON order. The last match wins; base arguments are defaults; unmatched conditional-only arguments are absent. Rule values must be strings, numbers, or booleans. +- Handoff: Ambiguity checks compare full step definitions including `conditional_arguments`, so entries with equal defaults but different rules do not collapse. Context never resolves category ambiguity. +- Handoff: Malformed rules raise `ArgumentError` at load with the `mappings[i].steps[j].conditional_arguments[k]` location. The shipped `scenario_measure_map.json` is unchanged and has no conditional rules yet. +- 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` - 29 examples, 0 failures. +- Verification: `C:\Ruby32-x64\bin\ruby.exe -S bundle exec rspec spec/tests/unit` - 91 examples, 0 failures. +- Verification: `bundle exec rubocop` on the changed Ruby files reports only `Metrics/BlockLength` on the mapper spec's top-level `describe` block. +- Follow-up: PR 11/12 should build the context from the reader once per file and pass it to `map_measure`. + ## 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_mapper.rb b/lib/BOSS/scenario_measure_mapper.rb index e2161ff..5c3a27b 100644 --- a/lib/BOSS/scenario_measure_mapper.rb +++ b/lib/BOSS/scenario_measure_mapper.rb @@ -17,7 +17,14 @@ module BOSS # 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. + # + # A step may carry conditional_arguments: an ordered list of { when:, arguments: } rules. + # A rule applies when every `when` key matches the caller's context; matching rules merge + # into the step arguments in order, so the last match wins. class ScenarioMeasureMapper + CONDITION_KEYS = [:building_type, :hvac_system_type].freeze + RULE_KEYS = [:when, :arguments].freeze + def initialize(map_path = SCENARIO_MEASURE_MAP_PATH) @map_path = map_path mappings = _load_mappings(map_path) @@ -26,8 +33,12 @@ def initialize(map_path = SCENARIO_MEASURE_MAP_PATH) end # @param measure [Hash] symbol-keyed measure hash from the reader + # @param context [Hash, nil] { building_type:, hvac_system_type: } used by conditional_arguments # @return [Hash] { measure_id:, steps: [ { measure_dir_name:, arguments: {} } ], warnings: [] } - def map_measure(measure) + def map_measure(measure, context: {}) + context ||= {} + raise ArgumentError, 'context must be a Hash' if !context.is_a?(Hash) + 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]) @@ -58,7 +69,7 @@ def map_measure(measure) return { measure_id: measure[:measure_id], - steps: _deep_copy(mapping[:steps]), + steps: mapping[:steps].map { |step| _effective_step(step, context) }, warnings: [] } end @@ -77,9 +88,75 @@ def _load_mappings(map_path) mappings = map.is_a?(Hash) ? map[:mappings] : nil raise ArgumentError, "Scenario measure map '#{map_path}' must define a 'mappings' array" if !mappings.is_a?(Array) + mappings.each_with_index do |mapping, mapping_index| + Array(mapping.is_a?(Hash) ? mapping[:steps] : nil).each_with_index do |step, step_index| + next if !step.is_a?(Hash) || !step.key?(:conditional_arguments) + + location = "Scenario measure map '#{map_path}' mappings[#{mapping_index}].steps[#{step_index}].conditional_arguments" + _validate_conditional_arguments(step[:conditional_arguments], location) + end + end + return mappings end + def _validate_conditional_arguments(rules, location) + raise ArgumentError, "#{location} must be an array" if !rules.is_a?(Array) + + rules.each_with_index do |rule, rule_index| + rule_location = "#{location}[#{rule_index}]" + raise ArgumentError, "#{rule_location} must be an object" if !rule.is_a?(Hash) + + unknown_keys = rule.keys - RULE_KEYS + raise ArgumentError, "#{rule_location} has unknown keys #{unknown_keys.inspect}" if unknown_keys.any? + + predicates = rule[:when] + if !predicates.is_a?(Hash) || predicates.empty? + raise ArgumentError, "#{rule_location}.when must be a non-empty object" + end + + predicates.each do |key, value| + raise ArgumentError, "#{rule_location}.when has unknown key '#{key}'" if !CONDITION_KEYS.include?(key) + + candidates = value.is_a?(Array) ? value : [value] + if candidates.empty? || candidates.any? { |candidate| !candidate.is_a?(String) || _normalize(candidate).nil? } + raise ArgumentError, "#{rule_location}.when.#{key} must be a non-blank string or non-empty array of them" + end + end + + arguments = rule[:arguments] + if !arguments.is_a?(Hash) || arguments.empty? + raise ArgumentError, "#{rule_location}.arguments must be a non-empty object" + end + + arguments.each do |name, value| + raise ArgumentError, "#{rule_location}.arguments has a blank name" if _normalize(name).nil? + if [String, Numeric, TrueClass, FalseClass].none? { |type| value.is_a?(type) } + raise ArgumentError, "#{rule_location}.arguments.#{name} must be a string, number, or boolean" + end + end + end + end + + def _effective_step(step, context) + effective_step = _deep_copy(step) + rules = effective_step.delete(:conditional_arguments) || [] + rules.each do |rule| + (effective_step[:arguments] ||= {}).merge!(rule[:arguments]) if _rule_matches?(rule, context) + end + + return effective_step + end + + def _rule_matches?(rule, context) + return rule[:when].all? do |key, expected| + actual = context[key] + next false if !actual.is_a?(String) || _normalize(actual).nil? + + Array(expected).any? { |candidate| _normalize(candidate) == _normalize(actual) } + end + end + # 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) diff --git a/spec/tests/unit/scenario_measure_mapper_spec.rb b/spec/tests/unit/scenario_measure_mapper_spec.rb index d1610a0..7c2a9a6 100644 --- a/spec/tests/unit/scenario_measure_mapper_spec.rb +++ b/spec/tests/unit/scenario_measure_mapper_spec.rb @@ -330,7 +330,164 @@ def lighting_measure(overrides = {}) end end + describe 'map_measure with conditional_arguments should' do + def conditional_step(measure_dir_name, rules, arguments = { __SKIP__: false }) + { measure_dir_name:, arguments:, conditional_arguments: rules } + end + + def conditional_mapping(steps, system_category_affected: 'Lighting', technology_category_element_name: 'LightingImprovements') + { + system_category_affected:, + technology_category_element_name:, + measure_name: 'Conditional measure', + steps: + } + end + + def conditional_mapper(mappings) + with_map_file(JSON.generate(mappings:)) { |map_path| yield BOSS::ScenarioMeasureMapper.new(map_path) } + end + + let(:conditional_measure) { lighting_measure(measure_name: 'Conditional measure') } + let(:rules) do + [ + { when: { building_type: ['SmallOffice', 'MediumOffice'] }, arguments: { space_type: 'Office' } }, + { when: { building_type: 'SmallOffice', hvac_system_type: 'PSZ-HP' }, arguments: { space_type: 'Small HP office', heat_pump: true } }, + { when: { hvac_system_type: 'PSZ-HP' }, arguments: { lpd: 0, enabled: false } } + ] + end + + it 'include matching arguments and exclude non-matching ones' do + conditional_mapper([conditional_mapping([conditional_step('Conditional', rules, { __SKIP__: false, lpd: 0.6 })])]) do |map_mapper| + cases = { + nil => { __SKIP__: false, lpd: 0.6 }, + { building_type: 'MediumOffice' } => { __SKIP__: false, lpd: 0.6, space_type: 'Office' }, + { building_type: ' smalloffice ', hvac_system_type: 'psz-hp', extra: 'ignored' } => + { __SKIP__: false, lpd: 0, space_type: 'Small HP office', heat_pump: true, enabled: false }, + { building_type: 'SmallOffice', hvac_system_type: 'PSZ-AC with gas unit heaters' } => + { __SKIP__: false, lpd: 0.6, space_type: 'Office' }, + { building_type: 'RetailStandalone', hvac_system_type: 'PSZ-HP' } => { __SKIP__: false, lpd: 0, enabled: false }, + { building_type: nil, hvac_system_type: '' } => { __SKIP__: false, lpd: 0.6 }, + { building_type: :SmallOffice } => { __SKIP__: false, lpd: 0.6 } + } + + cases.each do |context, expected_arguments| + result = map_mapper.map_measure(conditional_measure, context:) + expect(result).to eq(measure_id: 'Measure1', steps: [{ measure_dir_name: 'Conditional', arguments: expected_arguments }], warnings: []) + end + expect(map_mapper.map_measure(conditional_measure)[:steps].first[:arguments]).to eq(__SKIP__: false, lpd: 0.6) + end + end + + it 'apply rules per step, keep step order, and drop metadata' do + steps = [ + conditional_step('First', [{ when: { building_type: 'SmallOffice' }, arguments: { first: 1 } }]), + conditional_step('Second', []), + conditional_step('Third', [{ when: { building_type: 'LargeOffice' }, arguments: { third: 'x' } }]) + ] + + conditional_mapper([conditional_mapping(steps)]) do |map_mapper| + result = map_mapper.map_measure(conditional_measure, context: { building_type: 'SmallOffice' }) + + expect(result[:steps]).to eq [ + { measure_dir_name: 'First', arguments: { __SKIP__: false, first: 1 } }, + { measure_dir_name: 'Second', arguments: { __SKIP__: false } }, + { measure_dir_name: 'Third', arguments: { __SKIP__: false } } + ] + end + end + + it 'not mutate inputs or leak state between calls' do + conditional_mapper([conditional_mapping([conditional_step('Conditional', rules)])]) do |map_mapper| + context = { building_type: 'SmallOffice', hvac_system_type: 'PSZ-HP' } + context_before = Marshal.load(Marshal.dump(context)) + + first = map_mapper.map_measure(conditional_measure, context:) + first[:steps].first[:arguments][:space_type] << ' changed' + first[:steps].first[:arguments][:extra] = 1 + + expect(context).to eq context_before + expect(map_mapper.map_measure(conditional_measure, context:)[:steps].first[:arguments][:space_type]).to eq 'Small HP office' + expect(map_mapper.map_measure(conditional_measure, context: { building_type: 'LargeOffice' })[:steps].first[:arguments]).to eq(__SKIP__: false) + end + end + + it 'raise ArgumentError when context is not a Hash' do + expect { mapper.map_measure(lighting_measure, context: 'SmallOffice') }.to raise_error(ArgumentError, /context must be a Hash/) + end + + it 'treat mappings with the same defaults but different rules as different mappings' do + wall = conditional_mapping( + [conditional_step('Shared', [{ when: { building_type: 'SmallOffice' }, arguments: { wall: true } }])], + system_category_affected: 'Wall', technology_category_element_name: 'BuildingEnvelopeModifications' + ) + roof = conditional_mapping( + [conditional_step('Shared', [{ when: { building_type: 'LargeOffice' }, arguments: { roof: true } }])], + system_category_affected: 'Roof', technology_category_element_name: 'BuildingEnvelopeModifications' + ) + envelope_measure = { measure_id: 'M', technology_category_element_name: 'BuildingEnvelopeModifications', measure_name: 'Conditional measure' } + + conditional_mapper([wall, roof, wall]) do |map_mapper| + ambiguous = map_mapper.map_measure(envelope_measure, context: { building_type: 'MediumOffice' }) + resolved = map_mapper.map_measure(envelope_measure.merge(system_category_affected: 'Roof'), context: { building_type: 'LargeOffice' }) + + expect(ambiguous[:warnings].map { |warning| warning[:code] }).to eq [:ambiguous_technology_category_mapping] + expect(resolved[:steps]).to eq [{ measure_dir_name: 'Shared', arguments: { __SKIP__: false, roof: true } }] + end + end + + it 'match context derived from the reader for example-smalloffice-level1' do + xml_path = mapper_fixture_path('v2.7.0', 'example-smalloffice-level1.xml') + doc = REXML::Document.new(File.read(xml_path), ignore_whitespace_nodes: :all) + reader = BOSS::BuildingSyncReader.new(doc, nil, ASHRAE90_1) + allow(BOSS.BOSS_logger).to receive(:info) + building_type, = reader.get_building_type_and_bar_division_method + context = { building_type:, hvac_system_type: reader.get_principal_HVAC_system_type } + expect(context).to eq(building_type: 'SmallOffice', hvac_system_type: 'PSZ-HP') + + conditional_mapper([conditional_mapping([conditional_step('Conditional', rules)])]) do |map_mapper| + expect(map_mapper.map_measure(conditional_measure, context:)[:steps].first[:arguments]) + .to eq(__SKIP__: false, space_type: 'Small HP office', heat_pump: true, lpd: 0, enabled: false) + end + end + + it 'keep building_151 results unchanged when context is supplied' do + context = { building_type: 'RetailStandalone', hvac_system_type: 'VAV chiller with gas boiler reheat' } + building_151_measures.each_value do |measure| + expect(mapper.map_measure(measure, context:)).to eq mapper.map_measure(measure) + end + end + end + describe 'initialize should' do + it 'raise ArgumentError for malformed conditional_arguments' do + invalid_rule_sets = { + nil => /must be an array/, + 'rule' => /must be an array/, + ['rule'] => /\[0\] must be an object/, + [{ when: { building_type: 'A' }, arguments: { a: 1 }, unless: {} }] => /unknown keys/, + [{ arguments: { a: 1 } }] => /when must be a non-empty object/, + [{ when: {}, arguments: { a: 1 } }] => /when must be a non-empty object/, + [{ when: { occupancy: 'Office' }, arguments: { a: 1 } }] => /unknown key 'occupancy'/, + [{ when: { building_type: ' ' }, arguments: { a: 1 } }] => /when.building_type must be/, + [{ when: { building_type: [] }, arguments: { a: 1 } }] => /when.building_type must be/, + [{ when: { hvac_system_type: [1] }, arguments: { a: 1 } }] => /when.hvac_system_type must be/, + [{ when: { building_type: 'A' } }] => /arguments must be a non-empty object/, + [{ when: { building_type: 'A' }, arguments: {} }] => /arguments must be a non-empty object/, + [{ when: { building_type: 'A' }, arguments: { a: nil } }] => /arguments.a must be a string, number, or boolean/, + [{ when: { building_type: 'A' }, arguments: { a: [1] } }] => /arguments.a must be a string, number, or boolean/ + } + + invalid_rule_sets.each do |rule_set, message| + mapping = single_step_mapping('Lighting', 'LightingImprovements', 'Bad', 'Bad') + mapping[:steps].first[:conditional_arguments] = rule_set + with_map_file(JSON.generate(mappings: [mapping])) do |map_path| + expect { BOSS::ScenarioMeasureMapper.new(map_path) } + .to raise_error(ArgumentError, /mappings\[0\]\.steps\[0\]\.conditional_arguments.*#{message.source}/) + end + end + end + it 'raise ArgumentError when the map file does not exist' do expect { BOSS::ScenarioMeasureMapper.new(mapper_fixture_path('missing_map.json')) } .to raise_error(ArgumentError, /does not exist/)