Skip to content

Commit

Permalink
fix: Consistent probability sampler init (#1447)
Browse files Browse the repository at this point in the history
* fix: Consistent probability sampler init

* Math is hard

* Update sdk_experimental/test/opentelemetry/sdk/trace/samplers/consistent_probability_based_test.rb

Co-authored-by: Sam <370182+plantfansam@users.noreply.github.com>

* Appease the Handler

---------

Co-authored-by: Sam <370182+plantfansam@users.noreply.github.com>
  • Loading branch information
fbogsany and plantfansam authored May 15, 2023
1 parent d58c9bb commit f3e5f87
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def initialize(probability)
end

@p_floor = (Math.frexp(probability)[1] - 1).abs
@p_ceil = @p_floor + 1
@p_ceil = @p_floor - 1
floor = Math.ldexp(1.0, -@p_floor)
ceil = Math.ldexp(1.0, -@p_ceil)
@p_ceil_probability = (probability - floor) / (ceil - floor)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,54 @@

# TODO: statistical tests
end

describe '#initialize' do
# Check that the internal state is initialized correctly. We cache the
# floor and ceil of the negative power-of-two of the provided probability,
# and the probability of the ceil. It should be possible to calculate the
# original probability from these.
#
# Note that these are negative power-of-two exponent values, so the ceiling
# is the smaller value.

it 'initializes correctly for power-of-two probabilities' do
p_values = 62.downto(0)
probabilities = p_values.map { |i| 2**-i }
p_values.zip(probabilities).each do |p, probability|
sampler = OpenTelemetry::SDK::Trace::Samplers::ConsistentProbabilityBased.new(probability)
_(sampler.instance_variable_get(:@p_floor)).must_equal(p)
_(sampler.instance_variable_get(:@p_ceil)).must_equal(p-1)
_(sampler.instance_variable_get(:@p_ceil_probability)).must_equal(0)
end
end

it 'initializes correctly for 0' do
sampler = OpenTelemetry::SDK::Trace::Samplers::ConsistentProbabilityBased.new(0)
_(sampler.instance_variable_get(:@p_floor)).must_equal(63)
_(sampler.instance_variable_get(:@p_ceil)).must_equal(0)
_(sampler.instance_variable_get(:@p_ceil_probability)).must_equal(0)
end

it 'initializes correctly when the probability is close to a negative power-of-two' do
sampler = OpenTelemetry::SDK::Trace::Samplers::ConsistentProbabilityBased.new(0.12)
_(sampler.instance_variable_get(:@p_floor)).must_equal(4) # 2**-4 = 0.0625
_(sampler.instance_variable_get(:@p_ceil)).must_equal(3) # 2**-3 = 0.125
# Note obvious skew here: 0.12 is closer to 0.125 than 0.0625, so the probability of choosing the ceil is higher.
_(sampler.instance_variable_get(:@p_ceil_probability)).must_be_within_epsilon(0.92) # 0.0575 / 0.0625
end

it 'initializes correctly for 0.1' do
sampler = OpenTelemetry::SDK::Trace::Samplers::ConsistentProbabilityBased.new(0.1)
_(sampler.instance_variable_get(:@p_floor)).must_equal(4) # 2**-4 = 0.0625
_(sampler.instance_variable_get(:@p_ceil)).must_equal(3) # 2**-3 = 0.125
_(sampler.instance_variable_get(:@p_ceil_probability)).must_be_within_epsilon(0.6) # 0.0375 / 0.0625
ceil_prob = sampler.instance_variable_get(:@p_ceil_probability)
floor_prob = 1 - ceil_prob
p_floor = sampler.instance_variable_get(:@p_floor)
p_ceil = sampler.instance_variable_get(:@p_ceil)
# Verify that the sampling probability encoded in each `p` adds up to the probability
# in the the initializer argument
_((2**-p_ceil * ceil_prob) + (2**-p_floor * floor_prob)).must_be_within_epsilon(0.1)
end
end
end

0 comments on commit f3e5f87

Please sign in to comment.