diff --git a/app/controllers/api/v3/scenarios_controller.rb b/app/controllers/api/v3/scenarios_controller.rb index 6afd036d9..a4203101b 100644 --- a/app/controllers/api/v3/scenarios_controller.rb +++ b/app/controllers/api/v3/scenarios_controller.rb @@ -454,7 +454,7 @@ def export # Returns a ActionController::Parameters def filtered_params params.permit( - :autobalance, :force, :reset, gqueries: [] + :autobalance, :force, :force_balance, :reset, gqueries: [] ).merge(scenario: scenario_params) end diff --git a/app/controllers/inspect/checks_controller.rb b/app/controllers/inspect/checks_controller.rb index f519bf1bc..6a003c5e8 100644 --- a/app/controllers/inspect/checks_controller.rb +++ b/app/controllers/inspect/checks_controller.rb @@ -31,6 +31,7 @@ def inputs # @return [true, false] # Returns if the group sums up to -- or very close to -- 100. + # def ok? sum >= 99.9999 && sum <= 100.0001 end diff --git a/app/models/balancer.rb b/app/models/balancer.rb index edce50e54..ca4075b2c 100644 --- a/app/models/balancer.rb +++ b/app/models/balancer.rb @@ -1,17 +1,18 @@ # Balances a group of inputs so that the sum of their values "balances" to a # chosen number (typically 100). # -# Uses BigDecimal internally to prevent floating-point precision from causing -# minor imperfections in balanced values. +# Uses Rational arithmetic internally to prevent floating-point precision from +# causing minor imperfections in balanced values. # # Terminology # -# masters: -# Inputs whose value has been set by a user is called a "master". The -# balancer is not permitted to change these inputs. +# user_values: +# Inputs whose value has been set by a user is called a "user value". The +# balancer is not permitted to change these inputs, except when repairing +# drift (see INTENT_TOLERANCE). # # subordinates: -# Subordinates are all of the inputs in the group which are not masters. +# Subordinates are all of the inputs in the group which are not user_values. # The balancer will alter the values of these inputs in order that the # group sums to the equilibrium. # @@ -19,6 +20,13 @@ # The value to which all the inputs should sum. # class Balancer + # The intent tolerance: separates float drift from meaning. When Osmosis + # reports that a group cannot be balanced, a group whose total deviates from + # the equilibrium by no more than this is repaired by rescaling every member + # value-proportionally; a larger deviation cannot be distinguished from a + # typo and is refused. + INTENT_TOLERANCE = 1e-6 + # Creates a new Balancer instance. # # @params [Array] inputs @@ -32,7 +40,7 @@ class Balancer # def initialize(inputs, equilibrium = 100.0) @inputs = inputs - @equilibrium = equilibrium.to_d + @equilibrium = Rational(equilibrium.to_d) end # The name of the share group being balanced. @@ -41,7 +49,7 @@ def initialize(inputs, equilibrium = 100.0) # The group name. # def group_name - @inputs.any? ? @inputs.first.share_group.inspect : 'Unknown group' + @inputs.any? ? @inputs.first.share_group.to_s.inspect : 'Unknown group' end # A human-readable version of the Balancer. @@ -50,73 +58,160 @@ def group_name # Shows the Balancer group and equilibrium. # def inspect - "#" + "#" end # Balances the inputs. # - # Given one or more "master" inputs, whose values have been set explicitly + # Given one or more "user value" inputs, whose values have been set explicitly # by a user, all of the other "subordinate" inputs will have their values # changed. # # @param [Scenario] scenario # A scenario with an end year and area code, used to get the input # attributes. - # @param [HashInteger>] masters + # @param [HashInteger>] user_values # Inputs whose values have been set by the user, and should not be changed # by the balancer. + # @param [true, false] autobalance + # When false, every member is static: nothing may be moved to reach the + # equilibrium. A drift repair still applies — opting out of autobalancing + # is not opting into a rejection of data nobody mistyped. # # @return [Hash{Integer=>Numeric}] # Returns a hash containing values for the inputs whose values were not - # provided by the user. + # provided by the user. When a drift repair has occurred the hash also + # contains corrected values for user value keys: the values being corrected + # are the user's own, and +user_values+ wins everywhere it is read, so + # the repair must land there to take effect. # - def balance(scenario, user_values) + def balance(scenario, user_values, autobalance: true) # Remove inputs which aren't members of the group being balanced. user_values = user_values.slice(*@inputs.map(&:key)) - # We don't need to do anything if there are no masters. The group is at + # We don't need to do anything if there are no user_values. The group is at # the default values. return Hash.new if user_values.empty? - for_osmosis = @inputs.each_with_object({}) do |input, data| - data[input.key] = osmosis_hash(scenario, input, user_values[input.key]) - end - - balanced = Osmosis.balance(for_osmosis, @equilibrium) + members = members_for(scenario, user_values, autobalance) + balanced = Osmosis.balance(members, @equilibrium) # We return a hash containing the values for the subordinate inputs # converted to floats for convenient storage (Osmosis returns Rationals # which don't serialize so nicely into the +balanced_values+ column). balanced.each_with_object({}) do |(key, value), data| - data[key] = value.to_f unless user_values.key?(key) + data[key] = value.to_f unless members[key][:static] end rescue Osmosis::NoVariablesError - raise NoSubordinates.new(group_name, user_values) + repair_drift(scenario, members) || raise(NoSubordinates.new(group_name, user_values)) rescue Osmosis::CannotBalanceError - raise CannotBalance.new(group_name, user_values) + repair_drift(scenario, members) || raise(CannotBalance.new(group_name, user_values)) + end + + # The canonical value of each member of the group: the user's value if one + # is provided, otherwise the balanced value if one exists, otherwise the + # dataset default. Every input in the group is a member — a disabled input + # is not excluded (its slot keeps its default conversion, so a group summing + # the remaining members to the equilibrium would break energy conservation); + # it makes the group unresolvable instead (UnresolvableGroup). + # + # @return [Hash{String=>Numeric}] + def member_values(scenario, user_values, balanced_values = {}) + member_caches(scenario).each_with_object({}) do |(key, cache), values| + values[key] = user_values[key] || balanced_values[key] || cache[:default] + end + end + + # The members a value-proportional rescale of +values+ would push outside + # their own min/max. Lets the validator explain why a repair was refused + # instead of reporting a nonsensical "group sums to 100.0000000001". + # + # @return [Array] one hash per breach: key, rescaled value, min, max. + def repair_breaches(scenario, values) + breaches_in(scenario, rescaled_values(values)) end ####### private ####### - # Given an input, creates a hash which can be provided to Osmosis as one of - # the values in the group. - # - # @param [Input] input - # The input to be converted to an Osmosis-compatible hash. - # @param [Numeric, false] value - # Does this have a user-provided value for the input? If so, what is it? + # The cached attributes of every member of the group. A disabled member has + # no min/max/default — its value cannot be known, so neither can the + # group's balance — and makes the group unresolvable. + def member_caches(scenario) + @member_caches ||= @inputs.each_with_object({}) do |input, caches| + cache = Input.cache(scenario).read(scenario, input) + raise UnresolvableGroup.new(group_name, input.key, cache[:error]) if cache[:disabled] + + caches[input.key] = cache + end + end + + # The group's members as Osmosis elements. `static` means exactly one + # thing: this value may not be moved — true for values the user provided, + # and for every member when autobalancing is off. + def members_for(scenario, user_values, autobalance) + member_caches(scenario).each_with_object({}) do |(key, cache), members| + value = user_values[key] + + members[key] = { + min: cache[:min], + max: cache[:max], + value: value || cache[:default], + static: value.present? || !autobalance + } + end + end + + # Repairs drift: when Osmosis has ruled the group unbalanceable and the + # deviation from the equilibrium is within the intent tolerance, rescales + # every member value-proportionally (× equilibrium/total). This preserves + # the ratios between shares and leaves zero shares at exactly zero, which + # Osmosis's own delta-proportional rule would drive negative. # - # @return [Hash] - # Returns a Hash, ready for Osmosis. - def osmosis_hash(scenario, input, value) - cache = Input.cache.read(scenario, input) + # Returns nil — the caller re-raises — when the deviation is meaningful or + # a rescaled value would breach a member's bounds. + def repair_drift(scenario, members) + values = members.transform_values { |member| member[:value] } + deviation = (rational_sum(values) - @equilibrium).abs + + return nil if deviation > INTENT_TOLERANCE - { min: cache[:min], - max: cache[:max], - value: value || cache[:default], - static: value.present? || cache[:disabled] } + rescaled = rescaled_values(values) + return nil if breaches_in(scenario, rescaled).any? + + log_repair(scenario, deviation) + rescaled.transform_values(&:to_f) + end + + # The members of rescaled sitting outside their own min/max. + def breaches_in(scenario, rescaled) + caches = member_caches(scenario) + + rescaled.filter_map do |key, value| + cache = caches[key] + + unless value.between?(cache[:min], cache[:max]) + { key: key, value: value.to_f, min: cache[:min], max: cache[:max] } + end + end + end + + # The value-proportional rescale itself, exact in Rational. + def rescaled_values(values) + scale = @equilibrium / rational_sum(values) + values.transform_values { |value| Osmosis.rational(value) * scale } + end + + def rational_sum(values) + values.values.sum(Rational(0)) { |value| Osmosis.rational(value) } + end + + def log_repair(scenario, deviation) + Rails.logger.info( + "Repaired share-group drift: scenario=#{scenario.id} group=#{group_name} " \ + "deviation=#{deviation.to_f}" + ) end end # Balancer @@ -144,3 +239,20 @@ def message "with values #{ @values.inspect }" end end + +# An exception raised when a group contains a member whose value cannot be +# known (its input is disabled), making the group's balance unknowable. +class Balancer::UnresolvableGroup < Balancer::BalancerError + attr_reader :input_key, :cache_error + + def initialize(group, input_key, cache_error) + @group = group + @input_key = input_key + @cache_error = cache_error + end + + def message + "Cannot resolve group #{ @group }: the value of #{ @input_key } cannot " \ + "be determined (#{ @cache_error || 'input is disabled' })" + end +end diff --git a/app/models/scenario_updater.rb b/app/models/scenario_updater.rb index cce2d9dae..07c465562 100644 --- a/app/models/scenario_updater.rb +++ b/app/models/scenario_updater.rb @@ -54,11 +54,17 @@ def process(scenario_data, provided_values) autobalance = params[:autobalance] != 'false' && params[:autobalance] != false force_balance = params[:force_balance] - coupling_state = yield process_couplings(provided_values, active_couplings, uncouple) - user_values = yield calculate_user_values(provided_values, coupling_state, reset) - balanced_values = yield calculate_balanced_values( + coupling_state = yield process_couplings(provided_values, active_couplings, uncouple) + user_values = yield calculate_user_values(provided_values, coupling_state, reset) + + # Balancing may repair drift in the user's own values, so it returns the + # corrected user_values alongside the balanced values. + balance_state = yield calculate_balanced_values( user_values, provided_values, coupling_state, reset, autobalance, force_balance ) + user_values = balance_state[:user_values] + balanced_values = balance_state[:balanced_values] + _balanced = yield validate_balance(user_values, balanced_values, provided_values) Success([coupling_state, user_values, balanced_values]) diff --git a/app/models/scenario_updater/services/calculate_balanced_values.rb b/app/models/scenario_updater/services/calculate_balanced_values.rb index d09b6aba0..141d2aa81 100644 --- a/app/models/scenario_updater/services/calculate_balanced_values.rb +++ b/app/models/scenario_updater/services/calculate_balanced_values.rb @@ -3,46 +3,52 @@ class ScenarioUpdater module Services # Calculates balanced values for input share groups to ensure they sum to 100%. + # + # Balancing errors are swallowed here: the balancer computes, and + # ValidateBalance judges and reports, so exactly one service owns + # share-group error messages. class CalculateBalancedValues include Dry::Monads[:result] def call(scenario, user_values:, provided_values:, uncoupled_inputs:, reset: false, autobalance: true, force_balance: false) - return Success({}) if user_values.blank? + return Success(user_values:, balanced_values: {}) if user_values.blank? - balanced = base_balanced_values(scenario, uncoupled_inputs, reset) + user_values = user_values.dup + balanced = base_balanced_values(scenario, uncoupled_inputs, reset) - # Remove balanced values for groups being updated ShareGroups.each(provided_values) do |_, inputs| + # Remove balanced values for groups being updated. inputs.each { |input| balanced.delete(input.key) } - end - balance_groups(scenario, provided_values, user_values, autobalance, force_balance, balanced) if autobalance + corrections = balance_group( + scenario, inputs, user_values, provided_values, autobalance, force_balance + ) + + apply_corrections(corrections, user_values, balanced) + end - Success(balanced) + Success(user_values:, balanced_values: balanced) end private - def balance_groups(scenario, provided_values, user_values, autobalance, force_balance, balanced) - ShareGroups.each(provided_values) do |_, inputs| - if (balanced_group = balance_group(scenario, inputs, user_values, provided_values, force_balance)) - balanced.merge!(balanced_group) + # Corrections for keys the user set land in user_values; everything else + # is a balanced value. + def apply_corrections(corrections, user_values, balanced) + corrections.each do |key, value| + if user_values.key?(key) + user_values[key] = value + else + balanced[key] = value end end end - def balance_group(scenario, inputs, user_values, provided_values, force_balance) - if force_balance - values_to_balance = user_values.dup - inputs.each do |input| - values_to_balance.delete(input.key) unless provided_values.key?(input.key) - end - ::Balancer.new(inputs).balance(scenario, provided_values) - else - ::Balancer.new(inputs).balance(scenario, user_values) - end + def balance_group(scenario, inputs, user_values, provided_values, autobalance, force_balance) + values = force_balance ? provided_values : user_values + ::Balancer.new(inputs).balance(scenario, values, autobalance:) rescue ::Balancer::BalancerError - nil + {} end def base_balanced_values(scenario, uncoupled_inputs, reset) diff --git a/app/models/scenario_updater/services/validate_balance.rb b/app/models/scenario_updater/services/validate_balance.rb index e9983888b..f424d2719 100644 --- a/app/models/scenario_updater/services/validate_balance.rb +++ b/app/models/scenario_updater/services/validate_balance.rb @@ -2,11 +2,14 @@ class ScenarioUpdater module Services - # Validates that input share groups sum to 100% within an acceptable tolerance (0.01). + # Validates that input share groups sum to 100%. class ValidateBalance include Dry::Monads[:result] - TOLERANCE = 1.0E-12 + # The representation tolerance: absorbs the float re-summation of values + # that are exactly 100 as Rationals. + REPRESENTATION_TOLERANCE = 1.0E-12 + SHARE_GROUP_TOTAL = 1.0E2 def call(scenario, user_values:, balanced_values:, provided_values:, skip_validation: false) @@ -24,18 +27,42 @@ def call(scenario, user_values:, balanced_values:, provided_values:, skip_valida private def check_group_balance(group, inputs, scenario, user_values, balanced_values, errors) - values = inputs.map do |input| - input_cache = Input.cache(scenario).read(scenario, input) - next if input_cache[:disabled] + balancer = ::Balancer.new(inputs) + values = balancer.member_values(scenario, user_values, balanced_values) + deviation = (values.values.sum - SHARE_GROUP_TOTAL).abs + + return if deviation <= REPRESENTATION_TOLERANCE + + errors << group_error(balancer, scenario, values, deviation) + rescue ::Balancer::UnresolvableGroup => e + errors << "#{group.to_s.inspect} group cannot be resolved: the value of " \ + "#{e.input_key} cannot be determined (#{e.cache_error || 'input is disabled'})" + end + + def group_error(balancer, scenario, values, deviation) + breaches = + if deviation <= ::Balancer::INTENT_TOLERANCE + balancer.repair_breaches(scenario, values) + else + [] + end - user_values[input.key] || balanced_values[input.key] || input_cache[:default] - end.compact + return imbalance_error(balancer, values) if breaches.empty? - return if values.sum.between?(SHARE_GROUP_TOTAL - TOLERANCE, SHARE_GROUP_TOTAL + TOLERANCE) + "#{balancer.group_name} group sums to #{values.values.sum} and cannot be " \ + "repaired: #{breaches.map { |b| breach_message(b) }.join('; ')}" + end + + def imbalance_error(balancer, values) + info = values.map { |key, value| "#{key}=#{value}" }.join(' ') + + "#{balancer.group_name} group does not balance: group sums to " \ + "#{values.values.sum} using #{info}" + end - info = inputs.map(&:key).zip(values).map { |key, value| "#{key}=#{value}" }.join(' ') - errors << "#{group.to_s.inspect} group does not balance: group sums to " \ - "#{values.sum} using #{info}" + def breach_message(breach) + "rescaling #{breach[:key]} to #{breach[:value]} would move it outside " \ + "its bounds (#{breach[:min]}..#{breach[:max]})" end end end diff --git a/spec/fixtures/etsource/inputs/misc/broken_share_input.ad b/spec/fixtures/etsource/inputs/misc/broken_share_input.ad new file mode 100644 index 000000000..f0ae1f39d --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/broken_share_input.ad @@ -0,0 +1,9 @@ +# A share-group input whose start value GQL yields a non-numeric result +# (an area code string), so its cache is disabled with a cache error. A group +# containing it cannot be resolved and updates to the group must be refused. +- key = broken_share_input +- share_group = broken_group +- start_value_gql = present:AREA(area_code) +- min_value = 0 +- max_value = 100 +- query = 2 * 2 diff --git a/spec/fixtures/etsource/inputs/misc/broken_share_sibling.ad b/spec/fixtures/etsource/inputs/misc/broken_share_sibling.ad new file mode 100644 index 000000000..18f0d6f01 --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/broken_share_sibling.ad @@ -0,0 +1,8 @@ +# The healthy sibling of broken_share_input: updating it forces the +# broken_group share group to be balanced and validated. +- key = broken_share_sibling +- share_group = broken_group +- start_value = 100 +- min_value = 0 +- max_value = 100 +- query = 2 * 2 diff --git a/spec/fixtures/etsource/inputs/misc/grouped_input_five.ad b/spec/fixtures/etsource/inputs/misc/grouped_input_five.ad new file mode 100644 index 000000000..22bb40331 --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/grouped_input_five.ad @@ -0,0 +1,7 @@ +# An input which belongs to a group. +- key = grouped_input_five +- share_group = grouped +- start_value = 0 +- min_value = 0 +- max_value = 100 +- query = 2 * 2 diff --git a/spec/fixtures/etsource/inputs/misc/grouped_input_four.ad b/spec/fixtures/etsource/inputs/misc/grouped_input_four.ad new file mode 100644 index 000000000..3333bba4c --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/grouped_input_four.ad @@ -0,0 +1,7 @@ +# An input which belongs to a group. +- key = grouped_input_four +- share_group = grouped +- start_value = 0 +- min_value = 0 +- max_value = 100 +- query = 2 * 2 diff --git a/spec/fixtures/etsource/inputs/misc/grouped_input_seven.ad b/spec/fixtures/etsource/inputs/misc/grouped_input_seven.ad new file mode 100644 index 000000000..3ef36644e --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/grouped_input_seven.ad @@ -0,0 +1,9 @@ +# An input which belongs to a group. Its maximum is deliberately lower than +# the other members' so that a drift repair which rescales a value sitting at +# this maximum breaches its bounds (the repair must then be refused). +- key = grouped_input_seven +- share_group = grouped +- start_value = 0 +- min_value = 0 +- max_value = 50 +- query = 2 * 2 diff --git a/spec/fixtures/etsource/inputs/misc/grouped_input_six.ad b/spec/fixtures/etsource/inputs/misc/grouped_input_six.ad new file mode 100644 index 000000000..393b4dd21 --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/grouped_input_six.ad @@ -0,0 +1,7 @@ +# An input which belongs to a group. +- key = grouped_input_six +- share_group = grouped +- start_value = 0 +- min_value = 0 +- max_value = 100 +- query = 2 * 2 diff --git a/spec/fixtures/etsource/inputs/misc/grouped_input_three.ad b/spec/fixtures/etsource/inputs/misc/grouped_input_three.ad new file mode 100644 index 000000000..b5dea5544 --- /dev/null +++ b/spec/fixtures/etsource/inputs/misc/grouped_input_three.ad @@ -0,0 +1,7 @@ +# An input which belongs to a group. +- key = grouped_input_three +- share_group = grouped +- start_value = 0 +- min_value = 0 +- max_value = 100 +- query = 2 * 2 diff --git a/spec/models/scenario_updater/services/calculate_balanced_values_spec.rb b/spec/models/scenario_updater/services/calculate_balanced_values_spec.rb index d34e432b4..c54e5257d 100644 --- a/spec/models/scenario_updater/services/calculate_balanced_values_spec.rb +++ b/spec/models/scenario_updater/services/calculate_balanced_values_spec.rb @@ -6,7 +6,7 @@ let(:scenario) { FactoryBot.create(:scenario, balanced_values: { 'a' => 10 }) } let(:service) { described_class.new } - it 'returns Success with empty hash if user_values is blank' do + it 'returns Success with empty hashes if user_values is blank' do result = service.call( scenario, user_values: {}, @@ -17,7 +17,7 @@ force_balance: false ) expect(result).to be_success - expect(result.value!).to eq({}) + expect(result.value!).to eq(user_values: {}, balanced_values: {}) end it 'removes balanced values for groups being updated' do diff --git a/spec/models/scenario_updater/services/validate_balance_spec.rb b/spec/models/scenario_updater/services/validate_balance_spec.rb index 10341023b..36b7885ef 100644 --- a/spec/models/scenario_updater/services/validate_balance_spec.rb +++ b/spec/models/scenario_updater/services/validate_balance_spec.rb @@ -30,4 +30,36 @@ ) expect(result).to be_success end + + # Reachable when the balancer never saw these values -- force_balance + # balances the provided values while this service sums the user's, leaving + # within-tolerance drift that no repair ran on, and so no breach to name. + context 'with drift within the intent tolerance which no repair was attempted on' do + let(:values) do + { 'grouped_input_one' => 60.0, 'grouped_input_two' => 40.0000000001 } + end + + let(:result) do + service.call(scenario, user_values: values, balanced_values: {}, provided_values: values) + end + + it 'returns Failure' do + expect(result).to be_failure + end + + it 'reports the imbalance rather than trailing an empty explanation' do + expect(result.failure.first).to start_with( + '"grouped" group does not balance: group sums to 100.0000000001 using ' + ) + end + + it 'names every member and its value' do + expect(result.failure.first) + .to include('grouped_input_one=60.0', 'grouped_input_two=40.0000000001') + end + + it 'does not claim a repair was refused' do + expect(result.failure.first).not_to include('cannot be repaired') + end + end end diff --git a/spec/requests/api/v3/update_scenario_spec.rb b/spec/requests/api/v3/update_scenario_spec.rb index c486d6118..a4c65c265 100644 --- a/spec/requests/api/v3/update_scenario_spec.rb +++ b/spec/requests/api/v3/update_scenario_spec.rb @@ -94,6 +94,248 @@ def update_scenario(params: {}, headers: {}) end end + context 'when updating a share group' do + let(:user) { create(:user) } + let(:headers) { access_token_header(user, :write) } + let(:scenario) { FactoryBot.create(:scenario, user:) } + + let(:group_keys) do + %w[ + grouped_input_one grouped_input_two grouped_input_three + grouped_input_four grouped_input_five grouped_input_six + grouped_input_seven + ] + end + + + let(:drifting_values) do + { + 'grouped_input_one' => '53.28', + 'grouped_input_two' => '34.09', + 'grouped_input_three' => '12.6300000001', + 'grouped_input_four' => '0.0', + 'grouped_input_five' => '0.0', + 'grouped_input_six' => '0.0', + 'grouped_input_seven' => '0.0' + } + end + + def stored_group_sum + combined = scenario.user_values.merge(scenario.balanced_values || {}) + combined.slice(*group_keys).values.sum + end + + context 'with a fully-provided group drifting within the intent tolerance' do + let(:params) { { scenario: { user_values: drifting_values } } } + + it 'accepts the update' do + update_scenario(params:, headers:) + expect(response).to have_http_status(:ok) + end + + it 'stores the group summing to 100' do + update_scenario(params:, headers:) + expect(stored_group_sum).to be_within(1.0e-12).of(100.0) + end + + it 'keeps zero shares at exactly zero' do + update_scenario(params:, headers:) + + %w[grouped_input_four grouped_input_five grouped_input_six grouped_input_seven] + .each do |key| + expect(scenario.user_values[key]).to eq(0.0) + end + end + + it 'preserves the ratios between the shares' do + update_scenario(params:, headers:) + + expect(scenario.user_values['grouped_input_one'] / scenario.user_values['grouped_input_two']) + .to be_within(1.0e-12).of(53.28 / 34.09) + end + + it 'changes each value by less than the smallest expressible step' do + update_scenario(params:, headers:) + expect(scenario.user_values['grouped_input_one']).to be_within(1.0e-8).of(53.28) + end + + it 'logs the repair once, carrying the deviation' do + allow(Rails.logger).to receive(:info).and_call_original + update_scenario(params:, headers:) + + expect(Rails.logger).to have_received(:info) + .with(/Repaired share-group drift: scenario=#{scenario.id} .*deviation=1\.0e-10/) + .once + end + end + + context 'with a deviation of exactly the intent tolerance (1e-6)' do + let(:params) do + { scenario: { user_values: { + 'grouped_input_one' => '50.000001', + 'grouped_input_two' => '50.0', + 'grouped_input_three' => '0.0', + 'grouped_input_four' => '0.0', + 'grouped_input_five' => '0.0', + 'grouped_input_six' => '0.0', + 'grouped_input_seven' => '0.0' + } } } + end + + it 'accepts and repairs the group' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:ok) + expect(stored_group_sum).to be_within(1.0e-12).of(100.0) + end + end + + context 'with a deviation just above the intent tolerance (1.1e-6)' do + let(:params) do + { scenario: { user_values: { + 'grouped_input_one' => '50.0000011', + 'grouped_input_two' => '50.0', + 'grouped_input_three' => '0.0', + 'grouped_input_four' => '0.0', + 'grouped_input_five' => '0.0', + 'grouped_input_six' => '0.0', + 'grouped_input_seven' => '0.0' + } } } + end + + it 'refuses the update' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include('does not balance') + end + end + + context 'with a genuinely imbalanced group (50/30/10) and autobalance on' do + let(:params) do + { scenario: { user_values: { + 'grouped_input_one' => '50.0', + 'grouped_input_two' => '30.0', + 'grouped_input_three' => '10.0', + 'grouped_input_four' => '0.0', + 'grouped_input_five' => '0.0', + 'grouped_input_six' => '0.0', + 'grouped_input_seven' => '0.0' + } } } + end + + it 'refuses the update loudly' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include('does not balance') + end + end + + context 'with a drifting group where a zero share is omitted rather than provided' do + # Six of seven provided; the free member sits at its minimum of 0 and + # cannot absorb the negative excess (the CannotBalanceError path). + let(:params) do + { scenario: { user_values: drifting_values.except('grouped_input_seven') } } + end + + it 'accepts the update and repairs the group' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:ok) + expect(stored_group_sum).to be_within(1.0e-12).of(100.0) + end + + it 'keeps the omitted share at exactly zero, as a balanced value' do + update_scenario(params:, headers:) + expect(scenario.balanced_values['grouped_input_seven']).to eq(0.0) + end + end + + context 'with a drifting group and autobalance=false' do + let(:params) do + { autobalance: 'false', scenario: { user_values: drifting_values } } + end + + it 'accepts the update and repairs the group' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:ok) + expect(stored_group_sum).to be_within(1.0e-12).of(100.0) + end + end + + context 'when the repair would push a member outside its bounds' do + # grouped_input_seven sits at its maximum of 50; the group drifts low, so + # the rescale (× 100/99.9999999) would push it above the maximum. + let(:params) do + { scenario: { user_values: { + 'grouped_input_one' => '49.9999999', + 'grouped_input_two' => '0.0', + 'grouped_input_three' => '0.0', + 'grouped_input_four' => '0.0', + 'grouped_input_five' => '0.0', + 'grouped_input_six' => '0.0', + 'grouped_input_seven' => '50.0' + } } } + end + + it 'refuses the update, explaining the refused repair' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include('cannot be repaired') + expect(response.body).to include('grouped_input_seven') + expect(response.body).not_to include('does not balance') + end + end + + context 'when the group contains an input whose start value GQL is non-numeric' do + let(:params) do + { scenario: { user_values: { 'broken_share_sibling' => '100.0' } } } + end + + it 'refuses the update, naming the input and its cache error' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include('broken_share_input') + expect(response.body).to include('Non-numeric GQL value: default') + expect(response.body).not_to include('does not balance') + end + end + + context 'with force_balance' do + before do + scenario.update!(user_values: { + 'grouped_input_one' => 60.0, + 'grouped_input_two' => 40.0 + }) + end + + let(:params) do + { + force_balance: true, + scenario: { user_values: { 'grouped_input_one' => '50.0' } } + } + end + + it 'leaves values provided in the current request alone' do + update_scenario(params:, headers:) + + expect(response).to have_http_status(:ok) + expect(scenario.user_values['grouped_input_one']).to eq(50.0) + end + + it 'overwrites previously-set values to balance the group' do + update_scenario(params:, headers:) + + expect(scenario.user_values['grouped_input_two']).not_to eq(40.0) + expect(stored_group_sum).to be_within(1.0e-9).of(100.0) + end + end + end + context 'when a scenario has a version tag set by another user' do let(:params) { { scenario: { private: true } } } let(:user) { create(:user) } diff --git a/spec/support/input_memoization.rb b/spec/support/input_memoization.rb new file mode 100644 index 000000000..f7f8ef8b7 --- /dev/null +++ b/spec/support/input_memoization.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Input memoizes data derived from `all` -- share groups, coupling groups, +# before-update inputs -- in class-level instance variables which nothing +# invalidates. A spec which stubs `Input.all` and then triggers one of these +# lookups leaves the memo behind after RSpec unwinds the stub, so every later +# example in the process sees the stubbed subset. An empty `inputs_grouped` +# makes ScenarioUpdater::ShareGroups yield nothing, and share group validation +# silently passes without checking anything. +# +# Once the memos move into NastyCache (so `Etsource::Reloader` invalidates them +# together with `all` and `records`), this becomes `Input.clear!`. +module InputMemoizationHelper + DERIVED_MEMOS = %i[ + @before_inputs + @inputs_grouped + @coupling_inputs_keys + @coupling_groups + ].freeze + + def self.clear! + DERIVED_MEMOS.each { |memo| Input.instance_variable_set(memo, nil) } + end +end + +RSpec.configure do |config| + config.before { InputMemoizationHelper.clear! } +end