From eafb7517e3aac6e7cca49ba1b7bbb28ab3f4b2bc Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Mon, 31 Dec 2012 16:30:14 -0500 Subject: [PATCH 01/15] Refactor to allow for pluggable algorithm implementations. --- lib/split.rb | 2 +- lib/split/configuration.rb | 2 ++ lib/split/experiment.rb | 10 +--------- spec/experiment_spec.rb | 8 +++++++- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/split.rb b/lib/split.rb index e3b9649b..4ab790ff 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -1,4 +1,4 @@ -%w[experiment alternative helper version configuration persistence exceptions].each do |f| +%w[experiment alternative algorithms helper version configuration persistence exceptions].each do |f| require "split/#{f}" end diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index e1aaa978..839a76f7 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -22,6 +22,7 @@ class Configuration attr_accessor :enabled attr_accessor :persistence attr_accessor :experiments + attr_accessor :algorithm def initialize @robot_regex = /\b(#{BOTS.keys.join('|')})\b/i @@ -32,6 +33,7 @@ def initialize @allow_multiple_experiments = false @enabled = true @persistence = Split::Persistence::SessionAdapter + @algorithm = Split::Algorithms::WeightedSample end end end diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 27643a07..faa3182f 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -47,15 +47,7 @@ def next_alternative end def random_alternative - weights = alternatives.map(&:weight) - - total = weights.inject(:+) - point = rand * total - - alternatives.zip(weights).each do |n,w| - return n if w >= point - point -= w - end + Split.configuration.algorithm.choose_alternative(self) end def version diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index 9431ce63..c65de57b 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -150,8 +150,9 @@ end describe 'next_alternative' do + let(:experiment) { Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') } + it "should always return the winner if one exists" do - experiment = Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') green = Split::Alternative.new('green', 'link_color') experiment.winner = 'green' @@ -161,6 +162,11 @@ experiment = Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') experiment.next_alternative.name.should eql('green') end + + it "should use the specified algorithm if a winner does not exist" do + Split.configuration.algorithm.should_receive(:choose_alternative).and_return(Split::Alternative.new('green', 'link_color')) + experiment.next_alternative.name.should eql('green') + end end describe 'changing an existing experiment' do From 5d6b567fc7bc21c8cf83c7eb4cf642ace426143e Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Mon, 31 Dec 2012 17:39:19 -0500 Subject: [PATCH 02/15] pluggable experiment algorithms. Add a bandit implementation. --- lib/split.rb | 2 +- lib/split/algorithms.rb | 3 +++ lib/split/algorithms/weighted_sample.rb | 17 ++++++++++++ lib/split/algorithms/whiplash.rb | 35 +++++++++++++++++++++++++ lib/split/experiment.rb | 9 +++++++ spec/algorithms/weighted_sample_spec.rb | 13 +++++++++ spec/algorithms/whiplash_spec.rb | 18 +++++++++++++ spec/experiment_spec.rb | 14 ++++++++++ split.gemspec | 1 + 9 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 lib/split/algorithms.rb create mode 100644 lib/split/algorithms/weighted_sample.rb create mode 100644 lib/split/algorithms/whiplash.rb create mode 100644 spec/algorithms/weighted_sample_spec.rb create mode 100644 spec/algorithms/whiplash_spec.rb diff --git a/lib/split.rb b/lib/split.rb index 4ab790ff..fa4764ba 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -1,4 +1,4 @@ -%w[experiment alternative algorithms helper version configuration persistence exceptions].each do |f| +%w[algorithms experiment alternative helper version configuration persistence exceptions].each do |f| require "split/#{f}" end diff --git a/lib/split/algorithms.rb b/lib/split/algorithms.rb new file mode 100644 index 00000000..f7adf367 --- /dev/null +++ b/lib/split/algorithms.rb @@ -0,0 +1,3 @@ +%w[weighted_sample whiplash].each do |f| + require "split/algorithms/#{f}" +end \ No newline at end of file diff --git a/lib/split/algorithms/weighted_sample.rb b/lib/split/algorithms/weighted_sample.rb new file mode 100644 index 00000000..055d440d --- /dev/null +++ b/lib/split/algorithms/weighted_sample.rb @@ -0,0 +1,17 @@ +module Split + module Algorithms + module WeightedSample + def self.choose_alternative(experiment) + weights = experiment.alternatives.map(&:weight) + + total = weights.inject(:+) + point = rand * total + + experiment.alternatives.zip(weights).each do |n,w| + return n if w >= point + point -= w + end + end + end + end +end \ No newline at end of file diff --git a/lib/split/algorithms/whiplash.rb b/lib/split/algorithms/whiplash.rb new file mode 100644 index 00000000..f17742bd --- /dev/null +++ b/lib/split/algorithms/whiplash.rb @@ -0,0 +1,35 @@ +# A multi-armed bandit implementation inspired by +# @aaronsw and victorykit/whiplash +require 'simple-random' + +module Split + module Algorithms + module Whiplash + def self.choose_alternative(experiment) + experiment[best_guess(experiment.alternatives)] + end + + private + + def self.arm_guess(participants, completions) + a = [participants, 0].max + b = [participants-completions, 0].max + s = SimpleRandom.new; s.set_seed; s.beta(a+fairness_constant, b+fairness_constant) + end + + def self.best_guess(alternatives) + guesses = {} + alternatives.each do |alternative| + guesses[alternative.name] = arm_guess(alternative.participant_count, alternative.completed_count) + end + gmax = guesses.values.max + best = guesses.keys.select {|name| guesses[name] == gmax } + return best.sample + end + + def self.fairness_constant + 7 + end + end + end +end \ No newline at end of file diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index faa3182f..6eb1ab5d 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -1,6 +1,7 @@ module Split class Experiment attr_accessor :name + attr_writer :algorithm def initialize(name, *alternative_names) @name = name.to_s @@ -8,6 +9,10 @@ def initialize(name, *alternative_names) Split::Alternative.new(alternative, name) end end + + def algorithm + @algorithm ||= Split.configuration.algorithm + end def winner if w = Split.redis.hget(:experiment_winner, name) @@ -33,6 +38,10 @@ def start_time t = Split.redis.hget(:experiment_start_times, @name) Time.parse(t) if t end + + def [](name) + alternatives.find{|a| a.name == name} + end def alternatives @alternatives.dup diff --git a/spec/algorithms/weighted_sample_spec.rb b/spec/algorithms/weighted_sample_spec.rb new file mode 100644 index 00000000..8407ac7a --- /dev/null +++ b/spec/algorithms/weighted_sample_spec.rb @@ -0,0 +1,13 @@ +require "spec_helper" + +describe Split::Algorithms::WeightedSample do + it "should always return a heavily weighted option" do + experiment = Split::Experiment.find_or_create('link_color', {'blue' => 100}, {'red' => 0 }) + Split::Algorithms::WeightedSample.choose_alternative(experiment).name.should == 'blue' + end + + it "should return one of the results" do + experiment = Split::Experiment.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) + ['red', 'blue'].should include Split::Algorithms::WeightedSample.choose_alternative(experiment).name + end +end \ No newline at end of file diff --git a/spec/algorithms/whiplash_spec.rb b/spec/algorithms/whiplash_spec.rb new file mode 100644 index 00000000..fd43e4b7 --- /dev/null +++ b/spec/algorithms/whiplash_spec.rb @@ -0,0 +1,18 @@ +require "spec_helper" + +describe Split::Algorithms::Whiplash do + + it "should return one of the results" do + experiment = Split::Experiment.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) + ['red', 'blue'].should include Split::Algorithms::Whiplash.choose_alternative(experiment).name + end + + it "should guess floats" do + Split::Algorithms::Whiplash.send(:arm_guess, 0, 0).class.should == Float + Split::Algorithms::Whiplash.send(:arm_guess, 1, 0).class.should == Float + Split::Algorithms::Whiplash.send(:arm_guess, 2, 1).class.should == Float + Split::Algorithms::Whiplash.send(:arm_guess, 1000, 5).class.should == Float + Split::Algorithms::Whiplash.send(:arm_guess, 10, -2).class.should == Float + end + +end \ No newline at end of file diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index c65de57b..025dabd5 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'split/experiment' +require 'split/algorithms' describe Split::Experiment do it "should have a name" do @@ -148,6 +149,19 @@ experiment.version.should eql(1) end end + + describe 'algorithm' do + let(:experiment) { Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') } + + it 'should use the default algorithm if none is specified' do + experiment.algorithm.should == Split.configuration.algorithm + end + + it 'should use the user specified algorithm for this experiment if specified' do + experiment.algorithm = Split::Algorithms::Whiplash + experiment.algorithm.should == Split::Algorithms::Whiplash + end + end describe 'next_alternative' do let(:experiment) { Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') } diff --git a/split.gemspec b/split.gemspec index fe473679..01360ccb 100644 --- a/split.gemspec +++ b/split.gemspec @@ -21,6 +21,7 @@ Gem::Specification.new do |s| s.add_dependency 'redis', '>= 2.1' s.add_dependency 'redis-namespace', '>= 1.1.0' s.add_dependency 'sinatra', '>= 1.2.6' + s.add_dependency 'simple-random' # Ruby 1.8 doesn't include JSON in the std lib if RUBY_VERSION < "1.9" From 016d4dc64475a198f144533412f9670ce2475efa Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Mon, 31 Dec 2012 17:59:42 -0500 Subject: [PATCH 03/15] maintain ruby 1.8.7 compatibility. --- lib/split.rb | 2 +- lib/split/extensions.rb | 3 +++ lib/split/extensions/array.rb | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 lib/split/extensions.rb create mode 100644 lib/split/extensions/array.rb diff --git a/lib/split.rb b/lib/split.rb index fa4764ba..a4fda8d9 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -1,4 +1,4 @@ -%w[algorithms experiment alternative helper version configuration persistence exceptions].each do |f| +%w[algorithms extensions experiment alternative helper version configuration persistence exceptions].each do |f| require "split/#{f}" end diff --git a/lib/split/extensions.rb b/lib/split/extensions.rb new file mode 100644 index 00000000..28bd53fb --- /dev/null +++ b/lib/split/extensions.rb @@ -0,0 +1,3 @@ +%w[array].each do |f| + require "split/extensions/#{f}" +end \ No newline at end of file diff --git a/lib/split/extensions/array.rb b/lib/split/extensions/array.rb new file mode 100644 index 00000000..6e56dc3d --- /dev/null +++ b/lib/split/extensions/array.rb @@ -0,0 +1,4 @@ +class Array + # maintain backwards compatibility with 1.8.7 + alias_method :sample, :choice unless method_defined?(:sample) +end \ No newline at end of file From b83901a61f98ecb489ad41ce66f1a67b31e51afc Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Wed, 2 Jan 2013 19:21:32 -0500 Subject: [PATCH 04/15] WIP: Major changes to internals of Split to properly handle metrics, and configuration vs. redis storage. What a mess. Also tries to refactor helper.rb --- lib/split.rb | 2 +- lib/split/configuration.rb | 25 ++++ lib/split/exceptions.rb | 1 + lib/split/experiment.rb | 76 ++++++++++++- lib/split/extensions.rb | 2 +- lib/split/extensions/string.rb | 15 +++ lib/split/helper.rb | 201 +++++++++++++++++---------------- lib/split/metric.rb | 68 +++++++++++ lib/split/trial.rb | 38 +++++++ spec/configuration_spec.rb | 13 +++ spec/experiment_spec.rb | 22 ++++ spec/helper_spec.rb | 111 +++++++++--------- spec/metric_spec.rb | 30 +++++ spec/trial_spec.rb | 36 ++++++ split.gemspec | 2 + 15 files changed, 485 insertions(+), 157 deletions(-) create mode 100644 lib/split/extensions/string.rb create mode 100644 lib/split/metric.rb create mode 100644 lib/split/trial.rb create mode 100644 spec/metric_spec.rb create mode 100644 spec/trial_spec.rb diff --git a/lib/split.rb b/lib/split.rb index a4fda8d9..ada52dcc 100755 --- a/lib/split.rb +++ b/lib/split.rb @@ -1,4 +1,4 @@ -%w[algorithms extensions experiment alternative helper version configuration persistence exceptions].each do |f| +%w[algorithms extensions metric trial experiment alternative helper version configuration persistence exceptions].each do |f| require "split/#{f}" end diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index 839a76f7..1b4f6d5b 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -24,6 +24,31 @@ class Configuration attr_accessor :experiments attr_accessor :algorithm + def disabled? + !enabled + end + + def experiment_for(name) + if experiments + experiments[name] + end + end + + def metrics + return @metrics if defined?(@metrics) + @metrics = {} + if self.experiments + self.experiments.each do |key, value| + metric_name = value[:metric] + if metric_name + @metrics[metric_name] ||= [] + @metrics[metric_name] << Split::Experiment.load_from_configuration(key) + end + end + end + @metrics + end + def initialize @robot_regex = /\b(#{BOTS.keys.join('|')})\b/i @ignore_ip_addresses = [] diff --git a/lib/split/exceptions.rb b/lib/split/exceptions.rb index ea445682..f59995a7 100644 --- a/lib/split/exceptions.rb +++ b/lib/split/exceptions.rb @@ -1,3 +1,4 @@ module Split class InvalidPersistenceAdapterError < StandardError; end + class ExperimentNotFound < StandardError; end end \ No newline at end of file diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 6eb1ab5d..25df2e8e 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -2,16 +2,31 @@ module Split class Experiment attr_accessor :name attr_writer :algorithm + attr_accessor :resettable def initialize(name, *alternative_names) @name = name.to_s + @resettable = true @alternatives = alternative_names.map do |alternative| Split::Alternative.new(alternative, name) end end def algorithm - @algorithm ||= Split.configuration.algorithm + @algorithm ||= (load_algorithm || Split.configuration.algorithm) + end + + def ==(obj) + self.name == obj.name + end + + def load_algorithm + alg = Split.redis.hget(:experiment_algorithms, name) + if alg + alg.constantize + else + nil + end end def winner @@ -79,6 +94,10 @@ def finished_key "#{key}:finished" end + def resettable? + resettable + end + def reset alternatives.each(&:reset) reset_winner @@ -101,6 +120,7 @@ def save if new_record? Split.redis.sadd(:experiments, name) Split.redis.hset(:experiment_start_times, @name, Time.now) + Split.redis.hset(:experiment_algorithms, @name, algorithm.to_s) @alternatives.reverse.each {|a| Split.redis.lpush(name, a.name) } else Split.redis.del(name) @@ -109,6 +129,25 @@ def save end def self.load_alternatives_for(name) + if Split.configuration.experiment_for(name) + load_alternatives_from_configuration_for(name) + else + load_alternatives_from_redis_for(name) + end + end + + def self.load_alternatives_from_configuration_for(name) + alts = Split.configuration.experiment_for(name)[:variants] + if alts.is_a?(Hash) + alts.keys + else + alts + end + end + + + + def self.load_alternatives_from_redis_for(name) case Split.redis.type(name) when 'set' # convert legacy sets to lists alts = Split.redis.smembers(name) @@ -120,14 +159,43 @@ def self.load_alternatives_for(name) end end + def self.load_from_configuration(name) + obj = self.new(name, *load_alternatives_for(name)) + exp_config = Split.configuration.experiment_for(name) + if exp_config + obj.resettable = exp_config[:resettable] unless exp_config[:resettable].nil? + else + obj.resettable = true + end + obj + end + + def self.load_from_redis(name) + self.new(name, *load_alternatives_for(name)) + end + def self.all - Array(Split.redis.smembers(:experiments)).map {|e| find(e)} + Array(all_experiment_names_from_redis + all_experiment_names_from_configuration).map {|e| find(e)} + end + + def self.all_experiment_names_from_redis + Split.redis.smembers(:experiments) + end + + def self.all_experiment_names_from_configuration + Split.configuration.experiments ? Split.configuration.experiments.keys : [] end + def self.find(name) - if Split.redis.exists(name) - self.new(name, *load_alternatives_for(name)) + if Split.configuration.experiment_for(name) + obj = load_from_configuration(name) + elsif Split.redis.exists(name) + obj = load_from_redis(name) + else + obj = nil end + obj end def self.find_or_create(key, *alternatives) diff --git a/lib/split/extensions.rb b/lib/split/extensions.rb index 28bd53fb..4d13fd7c 100644 --- a/lib/split/extensions.rb +++ b/lib/split/extensions.rb @@ -1,3 +1,3 @@ -%w[array].each do |f| +%w[array string].each do |f| require "split/extensions/#{f}" end \ No newline at end of file diff --git a/lib/split/extensions/string.rb b/lib/split/extensions/string.rb new file mode 100644 index 00000000..20eba92a --- /dev/null +++ b/lib/split/extensions/string.rb @@ -0,0 +1,15 @@ +class String + # Constatntize is often provided by ActiveSupport, but ActiveSupport is not a dependency of Split. + unless method_defined?(:constantize) + def constantize + names = self.split('::') + names.shift if names.empty? || names.first.empty? + + constant = Object + names.each do |name| + constant = constant.const_defined?(name) ? constant.const_get(name) : constant.const_missing(name) + end + constant + end + end +end \ No newline at end of file diff --git a/lib/split/helper.rb b/lib/split/helper.rb index 36fdf4b4..e02ffd5a 100644 --- a/lib/split/helper.rb +++ b/lib/split/helper.rb @@ -1,20 +1,30 @@ module Split module Helper + def ab_test(experiment_name, control=nil, *alternatives) - if control.nil? && alternatives.length.zero? - experiment_config = Split.configuration.experiments[experiment_name] - unless experiment_config.nil? - control, alternatives = normalize_variants experiment_config[:variants] - end - elsif RUBY_VERSION.match(/1\.8/) && alternatives.length.zero? + if RUBY_VERSION.match(/1\.8/) && alternatives.length.zero? puts 'WARNING: You should always pass the control alternative through as the second argument with any other alternatives as the third because the order of the hash is not preserved in ruby 1.8' end - ret = if Split.configuration.enabled - experiment_variable(alternatives, control, experiment_name) - else - control_variable(control) - end + begin + ret = if Split.configuration.enabled + load_and_start_trial(experiment_name, control, alternatives) + else + control_variable(control) + end + + rescue => e + raise(e) unless Split.configuration.db_failover + Split.configuration.db_failover_on_db_error.call(e) + + if Split.configuration.db_failover_allow_parameter_override && override_present?(experiment_name) + ret = override_alternative(experiment_name) + end + ensure + if ret.nil? + ret = control_variable(control) + end + end if block_given? if defined?(capture) # a block in a rails view @@ -29,37 +39,34 @@ def ab_test(experiment_name, control=nil, *alternatives) end end - def finished(experiment_name, options = {:reset => true}) - return if exclude_visitor? or !Split.configuration.enabled + def reset!(experiment) + ab_user.delete(experiment.key) + end - experiment = Split::Experiment.find(experiment_name) - if experiment.nil? - return unless Split.configuration.experiments - Split.configuration.experiments.each do |name,exp| - if exp[:metric] == experiment_name - local_opts = {} - local_opts[:reset] = exp[:resettable] unless exp[:resettable].nil? - finished name, options.merge(local_opts) - end + def finish_experiment(experiment, options = {:reset => true}) + should_reset = experiment.resettable? && options[:reset] + if ab_user[experiment.finished_key] && !should_reset + return true + else + alternative_name = ab_user[experiment.key] + trial = Trial.new(experiment: experiment, alternative_name: alternative_name) + trial.complete! + if should_reset + reset!(experiment) + else + ab_user[experiment.finished_key] = true end - return - end - - if Split.configuration.experiments && Split.configuration.experiments[experiment_name] - reset = Split.configuration.experiments[experiment_name][:resettable] - options[:reset] = reset unless reset.nil? end + end - return if !options[:reset] && ab_user[experiment.finished_key] - if alternative_name = ab_user[experiment.key] - alternative = Split::Alternative.new(alternative_name, experiment_name) - alternative.increment_completion + def finished(metric_name, options = {:reset => true}) + return if exclude_visitor? || Split.configuration.disabled? + experiments = Metric.possible_experiments(metric_name) - if options[:reset] - ab_user.delete(experiment.key) - else - ab_user[experiment.finished_key] = true + if experiments.any? + experiments.each do |experiment| + finish_experiment(experiment, options) end end rescue => e @@ -67,13 +74,18 @@ def finished(experiment_name, options = {:reset => true}) Split.configuration.db_failover_on_db_error.call(e) end - def override(experiment_name, alternatives) - params[experiment_name] if defined?(params) && alternatives.include?(params[experiment_name]) + def override_present?(experiment_name) + defined?(params) && params[experiment_name] + end + + def override_alternative(experiment_name) + params[experiment_name] if override_present?(experiment_name) end def begin_experiment(experiment, alternative_name = nil) alternative_name ||= experiment.control.name ab_user[experiment.key] = alternative_name + alternative_name end def ab_user @@ -81,7 +93,7 @@ def ab_user end def exclude_visitor? - is_robot? or is_ignored_ip_address? + is_robot? || is_ignored_ip_address? end def not_allowed_to_test?(experiment_key) @@ -125,77 +137,70 @@ def control_variable(control) Hash === control ? control.keys.first : control end - def experiment_variable(alternatives, control, experiment_name) - begin + def load_and_start_trial(experiment_name, control, alternatives) + if control.nil? && alternatives.length.zero? + experiment = Experiment.find(experiment_name) + + raise ExperimentNotFound("#{experiment_name} not found") if experiment.nil? + else experiment = Split::Experiment.find_or_create(experiment_name, *([control] + alternatives)) - if experiment.winner - ret = experiment.winner.name + end + + start_trial( Trial.new(experiment: experiment) ) + end + + def start_trial(trial) + experiment = trial.experiment + if override_present?(experiment.name) + ret = override_alternative(experiment.name) + else + clean_old_versions(experiment) + if exclude_visitor? || not_allowed_to_test?(experiment.key) + ret = experiment.control.name else - if forced_alternative = override(experiment.name, experiment.alternative_names) - ret = forced_alternative + if ab_user[experiment.key] + ret = ab_user[experiment.key] else - clean_old_versions(experiment) - if exclude_visitor? or not_allowed_to_test?(experiment.key) - ret = experiment.control.name - else - if ab_user[experiment.key] - ret = ab_user[experiment.key] - else - alternative = experiment.next_alternative - alternative.increment_participation - begin_experiment(experiment, alternative.name) - ret = alternative.name - end - end + ret = begin_experiment(experiment, trial.alternative.name) end end - rescue => e - raise unless Split.configuration.db_failover - Split.configuration.db_failover_on_db_error.call(e) - if Split.configuration.db_failover_allow_parameter_override - all_alternatives = *([control] + alternatives) - alternative_names = all_alternatives.map{|a| a.is_a?(Hash) ? a.keys : a}.flatten - ret = override(experiment_name, alternative_names) - end - unless ret - ret = control_variable(control) - end end + ret end def keys_without_experiment(keys, experiment_key) keys.reject { |k| k.match(Regexp.new("^#{experiment_key}(:finished)?$")) } end - - def normalize_variants(variants) - given_probability, num_with_probability = variants.inject([0,0]) do |a,v| - p, n = a - if v.kind_of?(Hash) && v[:percent] - [p + v[:percent], n + 1] - else - a - end - end - - num_without_probability = variants.length - num_with_probability - unassigned_probability = ((100.0 - given_probability) / num_without_probability / 100.0) - - if num_with_probability.nonzero? - variants = variants.map do |v| - if v.kind_of?(Hash) && v[:name] && v[:percent] - { v[:name] => v[:percent] / 100.0 } - elsif v.kind_of?(Hash) && v[:name] - { v[:name] => unassigned_probability } - else - { v => unassigned_probability } - end - end - [variants.shift, variants.inject({}, :merge)] - else - [variants.shift, variants] - end - end + # + #def normalize_variants(variants) + # given_probability, num_with_probability = variants.inject([0,0]) do |a,v| + # p, n = a + # if v.kind_of?(Hash) && v[:percent] + # [p + v[:percent], n + 1] + # else + # a + # end + # end + # + # num_without_probability = variants.length - num_with_probability + # unassigned_probability = ((100.0 - given_probability) / num_without_probability / 100.0) + # + # if num_with_probability.nonzero? + # variants = variants.map do |v| + # if v.kind_of?(Hash) && v[:name] && v[:percent] + # { v[:name] => v[:percent] / 100.0 } + # elsif v.kind_of?(Hash) && v[:name] + # { v[:name] => unassigned_probability } + # else + # { v => unassigned_probability } + # end + # end + # [variants.shift, variants.inject({}, :merge)] + # else + # [variants.shift, variants] + # end + #end end end diff --git a/lib/split/metric.rb b/lib/split/metric.rb new file mode 100644 index 00000000..89b15ed6 --- /dev/null +++ b/lib/split/metric.rb @@ -0,0 +1,68 @@ +module Split + class Metric + attr_accessor :name + attr_accessor :experiments + + def initialize(attrs = {}) + attrs.each do |key,value| + if self.respond_to?("#{key}=") + self.send("#{key}=", value) + end + end + end + + def self.load_from_redis(name) + metric = Split.redis.hget(:metrics, name) + if metric + experiment_names = metric.split(',') + + experiments = experiment_names.collect do |experiment_name| + Split::Experiment.find(experiment_name) + end + + Split::Metric.new(name: name, experiments: experiments) + else + nil + end + end + + def self.load_from_configuration(name) + metrics = Split.configuration.metrics + if metrics && metrics[name] + Split::Metric.new(experiments: metrics[name], name: name) + else + nil + end + end + + def self.find(name) + name = name.intern + metric = load_from_configuration(name) + metric = load_from_redis(name) if metric.nil? + metric + end + + def self.possible_experiments(metric_name) + experiments = [] + metric = Split::Metric.find(metric_name) + if metric + experiments << metric.experiments + end + experiment = Split::Experiment.find(metric_name) + if experiment + experiments << experiment + end + experiments.flatten + end + + def save + Split.redis.hset(:metrics, name, experiments.map(&:name).join(',')) + end + + def complete! + experiments.each do |experiment| + experiment.complete! + end + end + end +end \ No newline at end of file diff --git a/lib/split/trial.rb b/lib/split/trial.rb new file mode 100644 index 00000000..4afa8e8d --- /dev/null +++ b/lib/split/trial.rb @@ -0,0 +1,38 @@ +module Split + class Trial + attr_accessor :experiment + attr_writer :alternative + + def initialize(attrs = {}) + attrs.each do |key,value| + if self.respond_to?("#{key}=") + self.send("#{key}=", value) + end + end + end + + def alternative + @alternative ||= select_alternative + end + + def complete! + alternative.increment_completion + end + + def alternative_name=(name) + self.alternative= experiment.alternatives.find{|a| a.name == name } + end + + private + + def select_alternative + if experiment.winner + experiment.winner + else + alt = experiment.next_alternative + alt.increment_participation + alt + end + end + end +end \ No newline at end of file diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 06c30c27..41d7549e 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -21,6 +21,11 @@ @config.enabled.should be_true end + it "disabled is the opposite of enabled" do + @config.enabled = false + @config.disabled?.should be_true + end + it "should provide a default pattern for robots" do %w[Baidu Gigabot Googlebot libwww-perl lwp-trivial msnbot SiteUptime Slurp WordPress ZIBB ZyBorg].each do |robot| @config.robot_regex.should =~ robot @@ -30,4 +35,12 @@ it "should use the session adapter for persistence by default" do @config.persistence.should eq(Split::Persistence::SessionAdapter) end + + it "should load a metric" do + @config.experiments = {:my_experiment=> + {:variants=>["control_opt", "other_opt"], :metric=>:my_metric}} + + @config.metrics.should_not be_nil + @config.metrics.keys.should == [:my_metric] + end end \ No newline at end of file diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index 025dabd5..26a0b77f 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -27,6 +27,15 @@ Split::Experiment.find('basket_text').start_time.should == experiment_start_time end + + it "should save the selected algorithm to redis" do + experiment_algorithm = Split::Algorithms::Whiplash + experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment.algorithm = experiment_algorithm + experiment.save + + Split::Experiment.find('basket_text').algorithm.should == experiment_algorithm + end it "should handle not having a start time" do experiment_start_time = Time.parse("Sat Mar 03 14:01:03") @@ -162,6 +171,19 @@ experiment.algorithm.should == Split::Algorithms::Whiplash end end + + describe 'load_algorithm' do + let(:experiment) { Split::Experiment.new('basket_text', 'Basket', "Cart") } + + it "should load an algorithm if it exists" do + Split.redis.hset(:experiment_algorithms, experiment.name, Split::Algorithms::Whiplash.to_s) + experiment.load_algorithm.should == Split::Algorithms::Whiplash + end + + it "should return nil if algorithm has not been persisted" do + experiment.load_algorithm.should + end + end describe 'next_alternative' do let(:experiment) { Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') } diff --git a/spec/helper_spec.rb b/spec/helper_spec.rb index 1221f80e..5f16f1e3 100755 --- a/spec/helper_spec.rb +++ b/spec/helper_spec.rb @@ -197,7 +197,8 @@ def should_finish_experiment(experiment_name, should_finish=true) alts = Split.configuration.experiments[experiment_name][:variants] experiment = Split::Experiment.find_or_create(experiment_name, *alts) alt_name = ab_user[experiment.key] = alts.first - alt = double + alt = mock('alternative') + alt.stub(:name).and_return(alt_name) Split::Alternative.stub(:new).with(alt_name, experiment_name).and_return(alt) if should_finish alt.should_receive(:increment_completion) @@ -578,58 +579,62 @@ def should_finish_experiment(experiment_name, should_finish=true) context "with preloaded config" do before { Split.configuration.experiments = {} } - it "pulls options from config file" do - Split.configuration.experiments[:my_experiment] = { - :variants => [ "control_opt", "other_opt" ], - } - should_receive(:experiment_variable).with(["other_opt"], "control_opt", :my_experiment) - ab_test :my_experiment - end - - it "accepts multiple variants" do - Split.configuration.experiments[:my_experiment] = { - :variants => [ "control_opt", "second_opt", "third_opt" ], - } - should_receive(:experiment_variable).with(["second_opt", "third_opt"], "control_opt", :my_experiment) - ab_test :my_experiment - end - - it "accepts probability on variants" do - Split.configuration.experiments[:my_experiment] = { - :variants => [ - { :name => "control_opt", :percent => 67 }, - { :name => "second_opt", :percent => 10 }, - { :name => "third_opt", :percent => 23 }, - ], - } - should_receive(:experiment_variable).with({"second_opt" => 0.1, "third_opt" => 0.23}, {"control_opt" => 0.67}, :my_experiment) - ab_test :my_experiment - end - - it "accepts probability on some variants" do - Split.configuration.experiments[:my_experiment] = { - :variants => [ - { :name => "control_opt", :percent => 34 }, - "second_opt", - { :name => "third_opt", :percent => 23 }, - "fourth_opt", - ], - } - should_receive(:experiment_variable).with({"second_opt" => 0.215, "third_opt" => 0.23, "fourth_opt" => 0.215}, {"control_opt" => 0.34}, :my_experiment) - ab_test :my_experiment - end - - it "allows name param without probability" do - Split.configuration.experiments[:my_experiment] = { - :variants => [ - { :name => "control_opt" }, - "second_opt", - { :name => "third_opt", :percent => 64 }, - ], - } - should_receive(:experiment_variable).with({"second_opt" => 0.18, "third_opt" => 0.64}, {"control_opt" => 0.18}, :my_experiment) - ab_test :my_experiment - end + #it "pulls options from config file" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ "control_opt", "other_opt" ], + # } + # + # trial = mock('trial') + # Split::Alternative.stub(:trial).with(experiment_name: :my_experiment).and_return(trial) + # should_receive(:start_trial).with(trial) + # + # ab_test :my_experiment + #end + # + #it "accepts multiple variants" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ "control_opt", "second_opt", "third_opt" ], + # } + # should_receive(:experiment_variable).with(["second_opt", "third_opt"], "control_opt", :my_experiment) + # ab_test :my_experiment + #end + + #it "accepts probability on variants" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ + # { :name => "control_opt", :percent => 67 }, + # { :name => "second_opt", :percent => 10 }, + # { :name => "third_opt", :percent => 23 }, + # ], + # } + # should_receive(:experiment_variable).with({"second_opt" => 0.1, "third_opt" => 0.23}, {"control_opt" => 0.67}, :my_experiment) + # ab_test :my_experiment + #end + # + #it "accepts probability on some variants" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ + # { :name => "control_opt", :percent => 34 }, + # "second_opt", + # { :name => "third_opt", :percent => 23 }, + # "fourth_opt", + # ], + # } + # should_receive(:experiment_variable).with({"second_opt" => 0.215, "third_opt" => 0.23, "fourth_opt" => 0.215}, {"control_opt" => 0.34}, :my_experiment) + # ab_test :my_experiment + #end + # + #it "allows name param without probability" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ + # { :name => "control_opt" }, + # "second_opt", + # { :name => "third_opt", :percent => 64 }, + # ], + # } + # should_receive(:experiment_variable).with({"second_opt" => 0.18, "third_opt" => 0.64}, {"control_opt" => 0.18}, :my_experiment) + # ab_test :my_experiment + #end end it 'should handle multiple experiments correctly' do diff --git a/spec/metric_spec.rb b/spec/metric_spec.rb new file mode 100644 index 00000000..18ea9e52 --- /dev/null +++ b/spec/metric_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' +require 'split/metric' + +describe Split::Metric do + describe 'possible experiments' do + it "should load the experiment if there is one, but no metric" do + experiment = Split::Experiment.find_or_create('color', 'red', 'blue') + Split::Metric.possible_experiments('color').should == [experiment] + end + + it "should load the experiments in a metric" do + experiment1 = Split::Experiment.find_or_create('color', 'red', 'blue') + experiment2 = Split::Experiment.find_or_create('size', 'big', 'small') + + metric = Split::Metric.new(name: 'purchase', experiments: [experiment1, experiment2]) + metric.save + Split::Metric.possible_experiments('purchase').should include(experiment1, experiment2) + end + + it "should load both the metric experiments and an experiment with the same name" do + experiment1 = Split::Experiment.find_or_create('purchase', 'red', 'blue') + experiment2 = Split::Experiment.find_or_create('size', 'big', 'small') + + metric = Split::Metric.new(name: 'purchase', experiments: [experiment2]) + metric.save + Split::Metric.possible_experiments('purchase').should include(experiment1, experiment2) + end + end + +end diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb new file mode 100644 index 00000000..95577cdc --- /dev/null +++ b/spec/trial_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' +require 'split/trial' + +describe Split::Trial do + it "should be initializeable" do + experiment = mock('experiment') + alternative = mock('alternative') + trial = Split::Trial.new(experiment: experiment, alternative: alternative) + trial.experiment.should == experiment + trial.alternative.should == alternative + end + + describe "alternative" do + it "should use the alternative if specified" do + trial = Split::Trial.new(experiment: experiment = mock('experiment'), alternative: alternative = mock('alternative')) + trial.should_not_receive(:select_alternative) + trial.alternative.should == alternative + end + + it "should call select_alternative if nil" do + trial = Split::Trial.new(experiment: experiment = mock('experiment')) + trial.should_receive(:select_alternative).and_return(alternative = mock('alternative')) + trial.alternative.should == alternative + end + end + + describe "alternative_name" do + it "should load the alternative when the alternative name is set" do + experiment = Split::Experiment.new('basket_text', 'basket', 'cart') + experiment.save + + trial = Split::Trial.new(experiment: experiment, alternative_name: 'basket') + trial.alternative.name.should == 'basket' + end + end +end diff --git a/split.gemspec b/split.gemspec index 01360ccb..141aa781 100644 --- a/split.gemspec +++ b/split.gemspec @@ -32,4 +32,6 @@ Gem::Specification.new do |s| s.add_development_dependency 'bundler', '~> 1.0' s.add_development_dependency 'rspec', '~> 2.12' s.add_development_dependency 'rack-test', '~> 0.6' + s.add_development_dependency 'pry' + s.add_development_dependency 'pry-debugger' end From d9500a154f3c04bbe92a9c65d03a268e6d27bae5 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 10:27:43 -0500 Subject: [PATCH 05/15] refactor trial model. --- lib/split/helper.rb | 1 + lib/split/trial.rb | 16 ++++++++++------ spec/trial_spec.rb | 6 ++++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/split/helper.rb b/lib/split/helper.rb index e02ffd5a..7aa5a50f 100644 --- a/lib/split/helper.rb +++ b/lib/split/helper.rb @@ -161,6 +161,7 @@ def start_trial(trial) if ab_user[experiment.key] ret = ab_user[experiment.key] else + trial.choose! ret = begin_experiment(experiment, trial.alternative.name) end end diff --git a/lib/split/trial.rb b/lib/split/trial.rb index 4afa8e8d..0c4a7aa5 100644 --- a/lib/split/trial.rb +++ b/lib/split/trial.rb @@ -1,7 +1,7 @@ module Split class Trial attr_accessor :experiment - attr_writer :alternative + attr_writer :alternative def initialize(attrs = {}) attrs.each do |key,value| @@ -12,20 +12,24 @@ def initialize(attrs = {}) end def alternative - @alternative ||= select_alternative + @alternative ||= if experiment.winner + experiment.winner + end end def complete! - alternative.increment_completion + alternative.increment_completion if alternative + end + + def choose! + self.alternative = choose end def alternative_name=(name) self.alternative= experiment.alternatives.find{|a| a.name == name } end - private - - def select_alternative + def choose if experiment.winner experiment.winner else diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb index 95577cdc..256dba68 100644 --- a/spec/trial_spec.rb +++ b/spec/trial_spec.rb @@ -13,13 +13,15 @@ describe "alternative" do it "should use the alternative if specified" do trial = Split::Trial.new(experiment: experiment = mock('experiment'), alternative: alternative = mock('alternative')) - trial.should_not_receive(:select_alternative) + trial.should_not_receive(:choose) trial.alternative.should == alternative end it "should call select_alternative if nil" do trial = Split::Trial.new(experiment: experiment = mock('experiment')) - trial.should_receive(:select_alternative).and_return(alternative = mock('alternative')) + trial.should_receive(:choose).and_return(alternative = mock('alternative')) + trial.choose! + trial.alternative.should == alternative end end From cc27ae004b9fef5979cca41c9e123e7cdb6da214 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 10:48:04 -0500 Subject: [PATCH 06/15] separate choosing, and recording. --- lib/split/trial.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/split/trial.rb b/lib/split/trial.rb index 0c4a7aa5..8c30c42a 100644 --- a/lib/split/trial.rb +++ b/lib/split/trial.rb @@ -23,6 +23,11 @@ def complete! def choose! self.alternative = choose + self.alternative.increment_participation + end + + def record! + self.alternative.increment_participation end def alternative_name=(name) @@ -34,7 +39,6 @@ def choose experiment.winner else alt = experiment.next_alternative - alt.increment_participation alt end end From 3669cad646847693140b66f6ad310ebdfa3d34ea Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 13:53:04 -0500 Subject: [PATCH 07/15] allow for single alternative experiments. --- lib/split/experiment.rb | 8 +++++--- spec/experiment_spec.rb | 11 ++++++++--- spec/trial_spec.rb | 1 + 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 25df2e8e..7fa28ed0 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -71,7 +71,11 @@ def next_alternative end def random_alternative - Split.configuration.algorithm.choose_alternative(self) + if alternatives.length > 1 + Split.configuration.algorithm.choose_alternative(self) + else + alternatives.first + end end def version @@ -204,8 +208,6 @@ def self.find_or_create(key, *alternatives) if alternatives.length == 1 if alternatives[0].is_a? Hash alternatives = alternatives[0].map{|k,v| {k => v} } - else - raise ArgumentError, 'You must declare at least 2 alternatives' end end diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index 26a0b77f..5abad5b5 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -204,6 +204,14 @@ experiment.next_alternative.name.should eql('green') end end + + describe 'single alternative' do + let(:experiment) { Split::Experiment.find_or_create('link_color', 'blue') } + + it "should always return the color blue" do + experiment.next_alternative.name.should eql('blue') + end + end describe 'changing an existing experiment' do it "should reset an experiment if it is loaded with different alternatives" do @@ -249,7 +257,4 @@ same_experiment.alternatives.map(&:weight).should == [1, 2] end end - - - end diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb index 256dba68..86feaa59 100644 --- a/spec/trial_spec.rb +++ b/spec/trial_spec.rb @@ -20,6 +20,7 @@ it "should call select_alternative if nil" do trial = Split::Trial.new(experiment: experiment = mock('experiment')) trial.should_receive(:choose).and_return(alternative = mock('alternative')) + alternative.should_receive(:increment_participation) trial.choose! trial.alternative.should == alternative From eef203e168add6850949e638a13f13a75eb84734 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 14:09:14 -0500 Subject: [PATCH 08/15] updates based on experience from app. --- lib/split/trial.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/split/trial.rb b/lib/split/trial.rb index 8c30c42a..f2d7f020 100644 --- a/lib/split/trial.rb +++ b/lib/split/trial.rb @@ -22,25 +22,24 @@ def complete! end def choose! - self.alternative = choose - self.alternative.increment_participation + choose + record! end def record! - self.alternative.increment_participation - end - - def alternative_name=(name) - self.alternative= experiment.alternatives.find{|a| a.name == name } + alternative.increment_participation end def choose if experiment.winner - experiment.winner + self.alternative = experiment.winner else - alt = experiment.next_alternative - alt + self.alternative = experiment.next_alternative end end + + def alternative_name=(name) + self.alternative= experiment.alternatives.find{|a| a.name == name } + end end end \ No newline at end of file From 804748e0b40f0371bc6a0fe0428820f709d06f59 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 16:29:40 -0500 Subject: [PATCH 09/15] memoize for performance. --- lib/split/alternative.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/split/alternative.rb b/lib/split/alternative.rb index 5828cc49..e50e976a 100644 --- a/lib/split/alternative.rb +++ b/lib/split/alternative.rb @@ -20,15 +20,16 @@ def to_s end def participant_count - Split.redis.hget(key, 'participant_count').to_i + @participant_count ||= Split.redis.hget(key, 'participant_count').to_i end def participant_count=(count) + @participant_count = count Split.redis.hset(key, 'participant_count', count.to_i) end def completed_count - Split.redis.hget(key, 'completed_count').to_i + @completed_count ||= Split.redis.hget(key, 'completed_count').to_i end def unfinished_count @@ -36,15 +37,16 @@ def unfinished_count end def completed_count=(count) + @completed_count = count Split.redis.hset(key, 'completed_count', count.to_i) end def increment_participation - Split.redis.hincrby key, 'participant_count', 1 + @participant_count = Split.redis.hincrby key, 'participant_count', 1 end def increment_completion - Split.redis.hincrby key, 'completed_count', 1 + @completed_count = Split.redis.hincrby key, 'completed_count', 1 end def control? @@ -91,6 +93,8 @@ def save end def reset + @participant_count = nil + @completed_count = nil Split.redis.hmset key, 'participant_count', 0, 'completed_count', 0 end From 295dc747699bdebd6c21477317f0de1a49bf710f Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 19:32:02 -0500 Subject: [PATCH 10/15] allow experiments to take options on initialization. --- lib/split/experiment.rb | 37 +++++---- spec/alternative_spec.rb | 14 ++-- spec/experiment_spec.rb | 169 +++++++++++++++++++++------------------ spec/trial_spec.rb | 8 +- 4 files changed, 125 insertions(+), 103 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 7fa28ed0..09e8374e 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -4,12 +4,23 @@ class Experiment attr_writer :algorithm attr_accessor :resettable - def initialize(name, *alternative_names) - @name = name.to_s - @resettable = true - @alternatives = alternative_names.map do |alternative| - Split::Alternative.new(alternative, name) - end + def initialize(name, options = {}) + options = { + :resettable => true, + }.merge(options) + + @name = name.to_s + @resettable = options[:resettable] if !options[:resettable].nil? + @algorithm = options[:algorithm] if !options[:algorithm].nil? + @alternatives = options[:alternatives] if !options[:alternatives].nil? + + if !options[:alternative_names].nil? + @alternatives = options[:alternative_names].map do |alternative| + Split::Alternative.new(alternative, name) + end + end + + end def algorithm @@ -149,8 +160,6 @@ def self.load_alternatives_from_configuration_for(name) end end - - def self.load_alternatives_from_redis_for(name) case Split.redis.type(name) when 'set' # convert legacy sets to lists @@ -164,7 +173,7 @@ def self.load_alternatives_from_redis_for(name) end def self.load_from_configuration(name) - obj = self.new(name, *load_alternatives_for(name)) + obj = self.new(name, :alternative_names => load_alternatives_for(name)) exp_config = Split.configuration.experiment_for(name) if exp_config obj.resettable = exp_config[:resettable] unless exp_config[:resettable].nil? @@ -175,7 +184,7 @@ def self.load_from_configuration(name) end def self.load_from_redis(name) - self.new(name, *load_alternatives_for(name)) + self.new(name, :alternative_names => load_alternatives_for(name)) end def self.all @@ -216,16 +225,16 @@ def self.find_or_create(key, *alternatives) if Split.redis.exists(name) existing_alternatives = load_alternatives_for(name) if existing_alternatives == alts.map(&:name) - experiment = self.new(name, *alternatives) + experiment = self.new(name, :alternative_names => alternatives) else - exp = self.new(name, *existing_alternatives) + exp = self.new(name, :alternative_names => existing_alternatives) exp.reset exp.alternatives.each(&:delete) - experiment = self.new(name, *alternatives) + experiment = self.new(name, :alternative_names =>alternatives) experiment.save end else - experiment = self.new(name, *alternatives) + experiment = self.new(name, :alternative_names => alternatives) experiment.save end return experiment diff --git a/spec/alternative_spec.rb b/spec/alternative_spec.rb index fc228650..32ba3cd4 100644 --- a/spec/alternative_spec.rb +++ b/spec/alternative_spec.rb @@ -4,13 +4,13 @@ describe Split::Alternative do it "should have a name" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) alternative = Split::Alternative.new('Basket', 'basket_text') alternative.name.should eql('Basket') end it "return only the name" do - experiment = Split::Experiment.new('basket_text', {'Basket' => 0.6}, {"Cart" => 0.4}) + experiment = Split::Experiment.new('basket_text', :alternative_names => [{'Basket' => 0.6}, {"Cart" => 0.4}]) alternative = Split::Alternative.new('Basket', 'basket_text') alternative.name.should eql('Basket') end @@ -26,7 +26,7 @@ end it "should belong to an experiment" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) experiment.save alternative = Split::Alternative.new('Basket', 'basket_text') alternative.experiment.name.should eql(experiment.name) @@ -39,7 +39,7 @@ end it "should increment participation count" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) experiment.save alternative = Split::Alternative.new('Basket', 'basket_text') old_participant_count = alternative.participant_count @@ -50,7 +50,7 @@ end it "should increment completed count" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) experiment.save alternative = Split::Alternative.new('Basket', 'basket_text') old_completed_count = alternative.participant_count @@ -70,7 +70,7 @@ end it "should know if it is the control of an experiment" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) experiment.save alternative = Split::Alternative.new('Basket', 'basket_text') alternative.control?.should be_true @@ -80,7 +80,7 @@ describe 'unfinished_count' do it "should be difference between participant and completed counts" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) experiment.save alternative = Split::Alternative.new('Basket', 'basket_text') alternative.increment_participation diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index 5abad5b5..9e1b2d04 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -1,64 +1,110 @@ require 'spec_helper' require 'split/experiment' require 'split/algorithms' +require 'time' describe Split::Experiment do - it "should have a name" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.name.should eql('basket_text') - end - - it "should have alternatives" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.alternatives.length.should be 2 - end + context "with an experiment" do + let(:experiment) { Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) } + + it "should have a name" do + experiment.name.should eql('basket_text') + end - it "should save to redis" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save - Split.redis.exists('basket_text').should be true - end + it "should have alternatives" do + experiment.alternatives.length.should be 2 + end + + it "should have alternatives with correct names" do + experiment.alternatives.collect{|a| a.name}.should == ['Basket', 'Cart'] + end + + it "should be resettable by default" do + experiment.resettable.should be_true + end + + it "should save to redis" do + experiment.save + Split.redis.exists('basket_text').should be true + end - it "should save the start time to redis" do - experiment_start_time = Time.parse("Sat Mar 03 14:01:03") - Time.stub(:now => experiment_start_time) - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save + it "should save the start time to redis" do + experiment_start_time = Time.parse("Sat Mar 03 14:01:03") + Time.stub(:now => experiment_start_time) + experiment.save - Split::Experiment.find('basket_text').start_time.should == experiment_start_time - end + Split::Experiment.find('basket_text').start_time.should == experiment_start_time + end - it "should save the selected algorithm to redis" do - experiment_algorithm = Split::Algorithms::Whiplash - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.algorithm = experiment_algorithm - experiment.save + it "should save the selected algorithm to redis" do + experiment_algorithm = Split::Algorithms::Whiplash + experiment.algorithm = experiment_algorithm + experiment.save - Split::Experiment.find('basket_text').algorithm.should == experiment_algorithm - end + Split::Experiment.find('basket_text').algorithm.should == experiment_algorithm + end - it "should handle not having a start time" do - experiment_start_time = Time.parse("Sat Mar 03 14:01:03") - Time.stub(:now => experiment_start_time) - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save + it "should handle not having a start time" do + experiment_start_time = Time.parse("Sat Mar 03 14:01:03") + Time.stub(:now => experiment_start_time) + experiment.save - Split.redis.hdel(:experiment_start_times, experiment.name) + Split.redis.hdel(:experiment_start_times, experiment.name) - Split::Experiment.find('basket_text').start_time.should == nil - end + Split::Experiment.find('basket_text').start_time.should == nil + end - it "should not create duplicates when saving multiple times" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save - experiment.save - Split.redis.exists('basket_text').should be true - Split.redis.lrange('basket_text', 0, -1).should eql(['Basket', "Cart"]) + it "should not create duplicates when saving multiple times" do + experiment.save + experiment.save + Split.redis.exists('basket_text').should be true + Split.redis.lrange('basket_text', 0, -1).should eql(['Basket', "Cart"]) + end + + describe 'new record?' do + it "should know if it hasn't been saved yet" do + experiment.new_record?.should be_true + end + + it "should know if it has been saved yet" do + experiment.save + experiment.new_record?.should be_false + end + end + + describe 'find' do + it "should return an existing experiment" do + experiment.save + Split::Experiment.find('basket_text').name.should eql('basket_text') + end + + it "should return an existing experiment" do + Split::Experiment.find('non_existent_experiment').should be_nil + end + end + + describe 'control' do + it 'should be the first alternative' do + experiment.save + experiment.control.name.should eql('Basket') + end + end end - + describe 'initialization' do + it "should set the algorithm when passed as an option to the initializer" do + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"], :algorithm => Split::Algorithms::Whiplash) + experiment.algorithm.should == Split::Algorithms::Whiplash + end + + it "should be possible to make an experiment not resettable" do + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"], :resettable => false) + experiment.resettable.should be_false + end + end + describe 'deleting' do it 'should delete itself' do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") + experiment = Split::Experiment.new('basket_text', alternative_names: [ 'Basket', "Cart"]) experiment.save experiment.delete @@ -74,39 +120,6 @@ end end - describe 'new record?' do - it "should know if it hasn't been saved yet" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.new_record?.should be_true - end - - it "should know if it has been saved yet" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save - experiment.new_record?.should be_false - end - end - - describe 'find' do - it "should return an existing experiment" do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save - Split::Experiment.find('basket_text').name.should eql('basket_text') - end - - it "should return an existing experiment" do - Split::Experiment.find('non_existent_experiment').should be_nil - end - end - - describe 'control' do - it 'should be the first alternative' do - experiment = Split::Experiment.new('basket_text', 'Basket', "Cart") - experiment.save - experiment.control.name.should eql('Basket') - end - end - describe 'winner' do it "should have no winner initially" do experiment = Split::Experiment.find_or_create('link_color', 'blue', 'red') @@ -173,7 +186,7 @@ end describe 'load_algorithm' do - let(:experiment) { Split::Experiment.new('basket_text', 'Basket', "Cart") } + let(:experiment) { Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) } it "should load an algorithm if it exists" do Split.redis.hset(:experiment_algorithms, experiment.name, Split::Algorithms::Whiplash.to_s) diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb index 2df4b272..4e2da4cd 100644 --- a/spec/trial_spec.rb +++ b/spec/trial_spec.rb @@ -5,14 +5,14 @@ it "should be initializeable" do experiment = mock('experiment') alternative = mock('alternative') - trial = Split::Trial.new(experiment: experiment, alternative: alternative) + trial = Split::Trial.new(:experiment => experiment, :alternative => alternative) trial.experiment.should == experiment trial.alternative.should == alternative end describe "alternative" do it "should use the alternative if specified" do - trial = Split::Trial.new(experiment: experiment = mock('experiment'), alternative: alternative = mock('alternative')) + trial = Split::Trial.new(:experiment => experiment = mock('experiment'), :alternative => alternative = mock('alternative')) trial.should_not_receive(:choose) trial.alternative.should == alternative end @@ -30,10 +30,10 @@ describe "alternative_name" do it "should load the alternative when the alternative name is set" do - experiment = Split::Experiment.new('basket_text', 'basket', 'cart') + experiment = Split::Experiment.new('basket_text', :alternative_names => ['basket', "cart"]) experiment.save - trial = Split::Trial.new(experiment: experiment, alternative_name: 'basket') + trial = Split::Trial.new(:experiment => experiment, :alternative_name => 'basket') trial.alternative.name.should == 'basket' end end From 4017ef8934c242ccf851bd4b7bee244a822e20b9 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 20:31:25 -0500 Subject: [PATCH 11/15] Store experiment configuration in redis. --- lib/split/experiment.rb | 51 ++++++++++++++++++++++------------------- spec/experiment_spec.rb | 38 +++++++++++++++++++----------- 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 09e8374e..1064160e 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -10,9 +10,15 @@ def initialize(name, options = {}) }.merge(options) @name = name.to_s - @resettable = options[:resettable] if !options[:resettable].nil? - @algorithm = options[:algorithm] if !options[:algorithm].nil? - @alternatives = options[:alternatives] if !options[:alternatives].nil? + @alternatives = options[:alternatives] if !options[:alternatives].nil? + + if !options[:algorithm].nil? + @algorithm = options[:algorithm].is_a?(String) ? options[:algorithm].constantize : options[:algorithm] + end + + if !options[:resettable].nil? + @resettable = options[:resettable].is_a?(String) ? options[:resettable] == 'true' : options[:resettable] + end if !options[:alternative_names].nil? @alternatives = options[:alternative_names].map do |alternative| @@ -20,25 +26,16 @@ def initialize(name, options = {}) end end - + end def algorithm - @algorithm ||= (load_algorithm || Split.configuration.algorithm) + @algorithm ||= Split.configuration.algorithm end def ==(obj) self.name == obj.name end - - def load_algorithm - alg = Split.redis.hget(:experiment_algorithms, name) - if alg - alg.constantize - else - nil - end - end def winner if w = Split.redis.hget(:experiment_winner, name) @@ -135,12 +132,15 @@ def save if new_record? Split.redis.sadd(:experiments, name) Split.redis.hset(:experiment_start_times, @name, Time.now) - Split.redis.hset(:experiment_algorithms, @name, algorithm.to_s) @alternatives.reverse.each {|a| Split.redis.lpush(name, a.name) } else Split.redis.del(name) @alternatives.reverse.each {|a| Split.redis.lpush(name, a.name) } end + config_key = Split::Experiment.experiment_config_key(name) + Split.redis.hset(config_key, :resettable, resettable) + Split.redis.hset(config_key, :algorithm, algorithm.to_s) + self end def self.load_alternatives_for(name) @@ -173,18 +173,21 @@ def self.load_alternatives_from_redis_for(name) end def self.load_from_configuration(name) - obj = self.new(name, :alternative_names => load_alternatives_for(name)) - exp_config = Split.configuration.experiment_for(name) - if exp_config - obj.resettable = exp_config[:resettable] unless exp_config[:resettable].nil? - else - obj.resettable = true - end - obj + exp_config = Split.configuration.experiment_for(name) || {} + self.new(name, :alternative_names => load_alternatives_for(name), + :resettable => exp_config[:resettable], + :algorithm => exp_config[:algorithm]) end def self.load_from_redis(name) - self.new(name, :alternative_names => load_alternatives_for(name)) + exp_config = Split.redis.hgetall(experiment_config_key(name)) + self.new(name, :alternative_names => load_alternatives_for(name), + :resettable => exp_config['resettable'], + :algorithm => exp_config['algorithm']) + end + + def self.experiment_config_key(name) + "experiment_configurations/#{name}" end def self.all diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index 9e1b2d04..dd9fa802 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -102,6 +102,31 @@ end end + describe 'persistent configuration' do + + it "should persist resettable in redis" do + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"], :resettable => false) + experiment.save + + e = Split::Experiment.find('basket_text') + e.should == experiment + e.resettable.should be_false + + end + + it "should persist algorithm in redis" do + experiment = Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"], :algorithm => Split::Algorithms::Whiplash) + experiment.save + + e = Split::Experiment.find('basket_text') + e.should == experiment + e.algorithm.should == Split::Algorithms::Whiplash + end + end + + + + describe 'deleting' do it 'should delete itself' do experiment = Split::Experiment.new('basket_text', alternative_names: [ 'Basket', "Cart"]) @@ -185,19 +210,6 @@ end end - describe 'load_algorithm' do - let(:experiment) { Split::Experiment.new('basket_text', :alternative_names => ['Basket', "Cart"]) } - - it "should load an algorithm if it exists" do - Split.redis.hset(:experiment_algorithms, experiment.name, Split::Algorithms::Whiplash.to_s) - experiment.load_algorithm.should == Split::Algorithms::Whiplash - end - - it "should return nil if algorithm has not been persisted" do - experiment.load_algorithm.should - end - end - describe 'next_alternative' do let(:experiment) { Split::Experiment.find_or_create('link_color', 'blue', 'red', 'green') } From e366dd7450521f8167ab52a4f49ef7c42ec3c98d Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 20:44:11 -0500 Subject: [PATCH 12/15] documentation for new features. --- README.mdown | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/README.mdown b/README.mdown index 008b0f60..2543254c 100644 --- a/README.mdown +++ b/README.mdown @@ -264,7 +264,7 @@ in a hash or a configuration file: config.experiments = YAML.load_file "config/experiments.yml" end -This hash can control your experiment's variants, weights, and if the +This hash can control your experiment's variants, weights, algorithm and if the experiment resets once finished: Split.configure do |config| @@ -274,6 +274,7 @@ experiment resets once finished: :resettable => false, }, :my_second_experiment => { + :algorithm => 'Split::Algorithms::Whiplash', :variants => [ { :name => "a", :percent => 67 }, { :name => "b", :percent => 33 }, @@ -310,6 +311,11 @@ Your code may then track a completion using the metric instead of the experiment name: finished(:conversion) + +You can also create a new metric by instantiating and saving a new Metric object. + + Split::Metric.new(:conversion) + Split::Metric.save ### DB failover solution @@ -377,6 +383,31 @@ Split.redis.namespace = "split:blog" We recommend sticking this in your initializer somewhere after Redis is configured. +## Outside of a Web Session + +Split provides the Helper module to facilitate running experiments inside web sessions. + +Alternatively, you can access the underlying Metric, Trial, Experiment and Alternative objects to +conduct experiments that are not tied to a web session. + +```ruby +# create a new experiment +experiment = Split::Experiment.find_or_create('color', 'red', 'blue') +# create a new trial +trial = Trial.new(:experiment => experiment) +# run trial +trial.choose! +# get the result +trial.alternative +# returns either red or blue +``` + +## Algorithms + +By default, Split ships with an algorithm that randomly selects from possible alternatives for a traditional a/b test. + +An implementation of a bandit algorithm is also provided. + ## Extensions - [Split::Export](http://github.com/andrew/split-export) - easily export ab test data out of Split From caf5a24d4525412ede88e5fa24baceeded5b4f21 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Thu, 3 Jan 2013 20:51:26 -0500 Subject: [PATCH 13/15] more documentation. --- README.mdown | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/README.mdown b/README.mdown index 2543254c..c634d1e5 100644 --- a/README.mdown +++ b/README.mdown @@ -397,9 +397,14 @@ experiment = Split::Experiment.find_or_create('color', 'red', 'blue') trial = Trial.new(:experiment => experiment) # run trial trial.choose! -# get the result -trial.alternative -# returns either red or blue +# get the result, returns either red or blue +trial.alternative.name + +# if the goal has been achieved, increment the successful completions for this alternative. +if goal_acheived? + trial.complete! +end + ``` ## Algorithms @@ -408,6 +413,8 @@ By default, Split ships with an algorithm that randomly selects from possible al An implementation of a bandit algorithm is also provided. +Users may also write their own algorithms. The default algorithm may be specified globally in the configuration file, or on a per experiment basis using the experiments hash of the configuration file. + ## Extensions - [Split::Export](http://github.com/andrew/split-export) - easily export ab test data out of Split From 2aa37c9281a109d2fe939031bd540856150842e0 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Fri, 4 Jan 2013 14:27:37 -0500 Subject: [PATCH 14/15] support ruby 1.8.7. --- lib/split/configuration.rb | 53 +++++++++++- lib/split/experiment.rb | 8 +- lib/split/helper.rb | 34 +------- lib/split/metric.rb | 6 +- lib/split/trial.rb | 10 +-- spec/alternative_spec.rb | 63 +++++++++++++++ spec/configuration_spec.rb | 20 +++++ spec/experiment_spec.rb | 2 +- spec/helper_spec.rb | 162 ++++++++++++++++++------------------- spec/metric_spec.rb | 4 +- spec/trial_spec.rb | 2 +- split.gemspec | 2 - 12 files changed, 229 insertions(+), 137 deletions(-) diff --git a/lib/split/configuration.rb b/lib/split/configuration.rb index 1b4f6d5b..218f2c27 100644 --- a/lib/split/configuration.rb +++ b/lib/split/configuration.rb @@ -20,8 +20,8 @@ class Configuration attr_accessor :db_failover_allow_parameter_override attr_accessor :allow_multiple_experiments attr_accessor :enabled - attr_accessor :persistence attr_accessor :experiments + attr_accessor :persistence attr_accessor :algorithm def disabled? @@ -29,8 +29,8 @@ def disabled? end def experiment_for(name) - if experiments - experiments[name] + if normalized_experiments + normalized_experiments[name] end end @@ -48,6 +48,52 @@ def metrics end @metrics end + + def normalized_experiments + if @experiments.nil? + nil + else + experiment_config = {} + @experiments.keys.each do | name | + experiment_config[name] = {} + end + @experiments.each do | experiment_name, settings| + experiment_config[experiment_name][:variants] = normalize_variants(settings[:variants]) if settings[:variants] + end + experiment_config + end + end + + + def normalize_variants(variants) + given_probability, num_with_probability = variants.inject([0,0]) do |a,v| + p, n = a + if v.kind_of?(Hash) && v[:percent] + [p + v[:percent], n + 1] + else + a + end + end + + num_without_probability = variants.length - num_with_probability + unassigned_probability = ((100.0 - given_probability) / num_without_probability / 100.0) + + if num_with_probability.nonzero? + variants = variants.map do |v| + if v.kind_of?(Hash) && v[:name] && v[:percent] + { v[:name] => v[:percent] / 100.0 } + elsif v.kind_of?(Hash) && v[:name] + { v[:name] => unassigned_probability } + else + { v => unassigned_probability } + end + end + [variants.shift, variants] + else + variants = variants.dup + [variants.shift, variants] + end + end def initialize @robot_regex = /\b(#{BOTS.keys.join('|')})\b/i @@ -57,6 +103,7 @@ def initialize @db_failover_allow_parameter_override = false @allow_multiple_experiments = false @enabled = true + @experiments = {} @persistence = Split::Persistence::SessionAdapter @algorithm = Split::Algorithms::WeightedSample end diff --git a/lib/split/experiment.rb b/lib/split/experiment.rb index 1064160e..d166c2c7 100644 --- a/lib/split/experiment.rb +++ b/lib/split/experiment.rb @@ -44,6 +44,10 @@ def winner nil end end + + def participant_count + alternatives.inject(0){|sum,a| sum + a.participant_count} + end def control alternatives.first @@ -156,7 +160,7 @@ def self.load_alternatives_from_configuration_for(name) if alts.is_a?(Hash) alts.keys else - alts + alts.flatten end end @@ -173,7 +177,7 @@ def self.load_alternatives_from_redis_for(name) end def self.load_from_configuration(name) - exp_config = Split.configuration.experiment_for(name) || {} + exp_config = Split.configuration.experiment_for(name) || {} self.new(name, :alternative_names => load_alternatives_for(name), :resettable => exp_config[:resettable], :algorithm => exp_config[:algorithm]) diff --git a/lib/split/helper.rb b/lib/split/helper.rb index 08ef601d..0f9860e0 100644 --- a/lib/split/helper.rb +++ b/lib/split/helper.rb @@ -49,7 +49,7 @@ def finish_experiment(experiment, options = {:reset => true}) return true else alternative_name = ab_user[experiment.key] - trial = Trial.new(experiment: experiment, alternative_name: alternative_name) + trial = Trial.new(:experiment => experiment, :alternative_name => alternative_name) trial.complete! if should_reset reset!(experiment) @@ -146,7 +146,7 @@ def load_and_start_trial(experiment_name, control, alternatives) experiment = Split::Experiment.find_or_create(experiment_name, *([control] + alternatives)) end - start_trial( Trial.new(experiment: experiment) ) + start_trial( Trial.new(:experiment => experiment) ) end def start_trial(trial) @@ -173,35 +173,5 @@ def start_trial(trial) def keys_without_experiment(keys, experiment_key) keys.reject { |k| k.match(Regexp.new("^#{experiment_key}(:finished)?$")) } end - - # def normalize_variants(variants) - # given_probability, num_with_probability = variants.inject([0,0]) do |a,v| - # p, n = a - # if v.kind_of?(Hash) && v[:percent] - # [p + v[:percent], n + 1] - # else - # a - # end - # end - # - # num_without_probability = variants.length - num_with_probability - # unassigned_probability = ((100.0 - given_probability) / num_without_probability / 100.0) - # - # if num_with_probability.nonzero? - # variants = variants.map do |v| - # if v.kind_of?(Hash) && v[:name] && v[:percent] - # { v[:name] => v[:percent] / 100.0 } - # elsif v.kind_of?(Hash) && v[:name] - # { v[:name] => unassigned_probability } - # else - # { v => unassigned_probability } - # end - # end - # [variants.shift, variants] - # else - # variants = variants.dup - # [variants.shift, variants] - # end - # end end end diff --git a/lib/split/metric.rb b/lib/split/metric.rb index 89b15ed6..5841a60c 100644 --- a/lib/split/metric.rb +++ b/lib/split/metric.rb @@ -20,7 +20,7 @@ def self.load_from_redis(name) Split::Experiment.find(experiment_name) end - Split::Metric.new(name: name, experiments: experiments) + Split::Metric.new(:name => name, :experiments => experiments) else nil end @@ -29,14 +29,14 @@ def self.load_from_redis(name) def self.load_from_configuration(name) metrics = Split.configuration.metrics if metrics && metrics[name] - Split::Metric.new(experiments: metrics[name], name: name) + Split::Metric.new(:experiments => metrics[name], :name => name) else nil end end def self.find(name) - name = name.intern + name = name.intern if name.is_a?(String) metric = load_from_configuration(name) metric = load_from_redis(name) if metric.nil? metric diff --git a/lib/split/trial.rb b/lib/split/trial.rb index f2d7f020..b5f1b68b 100644 --- a/lib/split/trial.rb +++ b/lib/split/trial.rb @@ -4,11 +4,9 @@ class Trial attr_writer :alternative def initialize(attrs = {}) - attrs.each do |key,value| - if self.respond_to?("#{key}=") - self.send("#{key}=", value) - end - end + self.experiment = attrs[:experiment] if !attrs[:experiment].nil? + self.alternative = attrs[:alternative] if !attrs[:alternative].nil? + self.alternative_name = attrs[:alternative_name] if !attrs[:alternative_name].nil? end def alternative @@ -39,7 +37,7 @@ def choose end def alternative_name=(name) - self.alternative= experiment.alternatives.find{|a| a.name == name } + self.alternative= self.experiment.alternatives.find{|a| a.name == name } end end end \ No newline at end of file diff --git a/spec/alternative_spec.rb b/spec/alternative_spec.rb index 32ba3cd4..8ac4d2d9 100644 --- a/spec/alternative_spec.rb +++ b/spec/alternative_spec.rb @@ -14,6 +14,69 @@ alternative = Split::Alternative.new('Basket', 'basket_text') alternative.name.should eql('Basket') end + + describe 'weights' do + + it "should set the weights" do + experiment = Split::Experiment.new('basket_text', :alternative_names => [{'Basket' => 0.6}, {"Cart" => 0.4}]) + first = experiment.alternatives[0] + first.name.should == 'Basket' + first.weight.should == 0.6 + + second = experiment.alternatives[1] + second.name.should == 'Cart' + second.weight.should == 0.4 + end + + it "accepts probability on variants" do + Split.configuration.experiments = { + :my_experiment => { + :variants => [ + { :name => "control_opt", :percent => 67 }, + { :name => "second_opt", :percent => 10 }, + { :name => "third_opt", :percent => 23 }, + ], + } + } + experiment = Split::Experiment.find(:my_experiment) + first = experiment.alternatives[0] + first.name.should == 'control_opt' + first.weight.should == 0.67 + + second = experiment.alternatives[1] + second.name.should == 'second_opt' + second.weight.should == 0.1 + end + + # it "accepts probability on some variants" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ + # { :name => "control_opt", :percent => 34 }, + # "second_opt", + # { :name => "third_opt", :percent => 23 }, + # "fourth_opt", + # ], + # } + # should start_experiment(:my_experiment).with({"control_opt" => 0.34}, {"second_opt" => 0.215}, {"third_opt" => 0.23}, {"fourth_opt" => 0.215}) + # ab_test :my_experiment + # end + # + # it "allows name param without probability" do + # Split.configuration.experiments[:my_experiment] = { + # :variants => [ + # { :name => "control_opt" }, + # "second_opt", + # { :name => "third_opt", :percent => 64 }, + # ], + # } + # should start_experiment(:my_experiment).with({"control_opt" => 0.18}, {"second_opt" => 0.18}, {"third_opt" => 0.64}) + # ab_test :my_experiment + # end + + it "should set the weights from a configuration file" do + + end + end it "should have a default participation count of 0" do alternative = Split::Alternative.new('Basket', 'basket_text') diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 41d7549e..fbc2a8e6 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -43,4 +43,24 @@ @config.metrics.should_not be_nil @config.metrics.keys.should == [:my_metric] end + + it "should allow loading of experiment using experment_for" do + @config.experiments = {:my_experiment=> + {:variants=>["control_opt", "other_opt"], :metric=>:my_metric}} + @config.experiment_for(:my_experiment).should == {:variants=>["control_opt", ["other_opt"]]} + end + + it "should normalize experiments" do + @config.experiments = { + :my_experiment => { + :variants => [ + { :name => "control_opt", :percent => 67 }, + { :name => "second_opt", :percent => 10 }, + { :name => "third_opt", :percent => 23 }, + ], + } + } + + @config.normalized_experiments.should == {:my_experiment=>{:variants=>[{"control_opt"=>0.67}, [{"second_opt"=>0.1}, {"third_opt"=>0.23}]]}} + end end \ No newline at end of file diff --git a/spec/experiment_spec.rb b/spec/experiment_spec.rb index dd9fa802..2ea72075 100644 --- a/spec/experiment_spec.rb +++ b/spec/experiment_spec.rb @@ -129,7 +129,7 @@ describe 'deleting' do it 'should delete itself' do - experiment = Split::Experiment.new('basket_text', alternative_names: [ 'Basket', "Cart"]) + experiment = Split::Experiment.new('basket_text', :alternative_names => [ 'Basket', "Cart"]) experiment.save experiment.delete diff --git a/spec/helper_spec.rb b/spec/helper_spec.rb index 94c940f7..8c0d8165 100755 --- a/spec/helper_spec.rb +++ b/spec/helper_spec.rb @@ -81,6 +81,8 @@ ab_test('link_color', {'blue' => 0.01}, 'red' => 0.2) experiment = Split::Experiment.find('link_color') experiment.alternative_names.should eql(['blue', 'red']) + # TODO: persist alternative weights + # experiment.alternatives.collect{|a| a.weight}.should == [0.01, 0.2] end it "should only let a user participate in one experiment at a time" do @@ -570,94 +572,84 @@ def should_finish_experiment(experiment_name, should_finish=true) finished('link_color') end end - - end - end - # context "with preloaded config" do - # before { Split.configuration.experiments = {} } - # - # subject { self } - # RSpec::Matchers.define :start_experiment do |name| - # match do |actual| - # @control ||= anything - # @alternatives ||= anything - # @times ||= 1 - # actual.should_receive(:experiment_variable).with(@alternatives, @control, name).exactly(@times).times - # end - # chain :with do |control, *alternatives| - # @control = control - # @alternatives = alternatives.flatten - # end - # chain :exactly do |times| - # @times = times - # end - # chain :times do end - # end - # - # it "pulls options from config file" do - # Split.configuration.experiments[:my_experiment] = { - # :variants => [ "control_opt", "other_opt" ], - # } - # should start_experiment(:my_experiment).with("control_opt", ["other_opt"]) - # ab_test :my_experiment - # end - # - # it "can be called multiple times" do - # Split.configuration.experiments[:my_experiment] = { - # :variants => [ "control_opt", "other_opt" ], - # } - # should start_experiment(:my_experiment).with("control_opt", ["other_opt"]).exactly(5).times - # 5.times { ab_test :my_experiment } - # end - # - # it "accepts multiple variants" do - # Split.configuration.experiments[:my_experiment] = { - # :variants => [ "control_opt", "second_opt", "third_opt" ], - # } - # should start_experiment(:my_experiment).with("control_opt", ["second_opt", "third_opt"]) - # ab_test :my_experiment - # end - # - # it "accepts probability on variants" do - # Split.configuration.experiments[:my_experiment] = { - # :variants => [ - # { :name => "control_opt", :percent => 67 }, - # { :name => "second_opt", :percent => 10 }, - # { :name => "third_opt", :percent => 23 }, - # ], - # } - # should start_experiment(:my_experiment).with({"control_opt" => 0.67}, {"second_opt" => 0.1}, {"third_opt" => 0.23}) - # ab_test :my_experiment - # end - # - # it "accepts probability on some variants" do - # Split.configuration.experiments[:my_experiment] = { - # :variants => [ - # { :name => "control_opt", :percent => 34 }, - # "second_opt", - # { :name => "third_opt", :percent => 23 }, - # "fourth_opt", - # ], - # } - # should start_experiment(:my_experiment).with({"control_opt" => 0.34}, {"second_opt" => 0.215}, {"third_opt" => 0.23}, {"fourth_opt" => 0.215}) - # ab_test :my_experiment - # end - # - # it "allows name param without probability" do - # Split.configuration.experiments[:my_experiment] = { - # :variants => [ - # { :name => "control_opt" }, - # "second_opt", - # { :name => "third_opt", :percent => 64 }, - # ], - # } - # should start_experiment(:my_experiment).with({"control_opt" => 0.18}, {"second_opt" => 0.18}, {"third_opt" => 0.64}) - # ab_test :my_experiment - # end - # end + context "with preloaded config" do + before { Split.configuration.experiments = {}} + + it "pulls options from config file" do + Split.configuration.experiments[:my_experiment] = { + :variants => [ "control_opt", "other_opt" ], + } + ab_test :my_experiment + Split::Experiment.find(:my_experiment).alternative_names.should == [ "control_opt", "other_opt" ] + end + + it "can be called multiple times" do + Split.configuration.experiments[:my_experiment] = { + :variants => [ "control_opt", "other_opt" ], + } + 5.times { ab_test :my_experiment } + experiment = Split::Experiment.find(:my_experiment) + experiment.alternative_names.should == [ "control_opt", "other_opt" ] + experiment.participant_count.should == 1 + end + + it "accepts multiple variants" do + Split.configuration.experiments[:my_experiment] = { + :variants => [ "control_opt", "second_opt", "third_opt" ], + } + ab_test :my_experiment + experiment = Split::Experiment.find(:my_experiment) + experiment.alternative_names.should == [ "control_opt", "second_opt", "third_opt" ] + end + + it "accepts probability on variants" do + Split.configuration.experiments[:my_experiment] = { + :variants => [ + { :name => "control_opt", :percent => 67 }, + { :name => "second_opt", :percent => 10 }, + { :name => "third_opt", :percent => 23 }, + ], + } + ab_test :my_experiment + experiment = Split::Experiment.find(:my_experiment) + experiment.alternatives.collect{|a| [a.name, a.weight]}.should == [['control_opt', 0.67], ['second_opt', 0.1], ['third_opt', 0.23]] + + end + + it "accepts probability on some variants" do + Split.configuration.experiments[:my_experiment] = { + :variants => [ + { :name => "control_opt", :percent => 34 }, + "second_opt", + { :name => "third_opt", :percent => 23 }, + "fourth_opt", + ], + } + ab_test :my_experiment + experiment = Split::Experiment.find(:my_experiment) + names_and_weights = experiment.alternatives.collect{|a| [a.name, a.weight]} + names_and_weights.should == [['control_opt', 0.34], ['second_opt', 0.215], ['third_opt', 0.23], ['fourth_opt', 0.215]] + names_and_weights.inject(0){|sum, nw| sum + nw[1]}.should == 1.0 + end + + it "allows name param without probability" do + Split.configuration.experiments[:my_experiment] = { + :variants => [ + { :name => "control_opt" }, + "second_opt", + { :name => "third_opt", :percent => 64 }, + ], + } + ab_test :my_experiment + experiment = Split::Experiment.find(:my_experiment) + names_and_weights = experiment.alternatives.collect{|a| [a.name, a.weight]} + names_and_weights.should == [['control_opt', 0.18], ['second_opt', 0.18], ['third_opt', 0.64]] + names_and_weights.inject(0){|sum, nw| sum + nw[1]}.should == 1.0 + end + end it 'should handle multiple experiments correctly' do experiment = Split::Experiment.find_or_create('link_color', 'blue', 'red') diff --git a/spec/metric_spec.rb b/spec/metric_spec.rb index 18ea9e52..dcbff4e0 100644 --- a/spec/metric_spec.rb +++ b/spec/metric_spec.rb @@ -12,7 +12,7 @@ experiment1 = Split::Experiment.find_or_create('color', 'red', 'blue') experiment2 = Split::Experiment.find_or_create('size', 'big', 'small') - metric = Split::Metric.new(name: 'purchase', experiments: [experiment1, experiment2]) + metric = Split::Metric.new(:name => 'purchase', :experiments => [experiment1, experiment2]) metric.save Split::Metric.possible_experiments('purchase').should include(experiment1, experiment2) end @@ -21,7 +21,7 @@ experiment1 = Split::Experiment.find_or_create('purchase', 'red', 'blue') experiment2 = Split::Experiment.find_or_create('size', 'big', 'small') - metric = Split::Metric.new(name: 'purchase', experiments: [experiment2]) + metric = Split::Metric.new(:name => 'purchase', :experiments => [experiment2]) metric.save Split::Metric.possible_experiments('purchase').should include(experiment1, experiment2) end diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb index 4e2da4cd..917b4ba7 100644 --- a/spec/trial_spec.rb +++ b/spec/trial_spec.rb @@ -18,7 +18,7 @@ end it "should call select_alternative if nil" do - trial = Split::Trial.new(experiment: experiment = mock('experiment')) + trial = Split::Trial.new(:experiment => experiment = mock('experiment')) experiment.should_receive(:next_alternative).and_return(alternative = mock('alternative')) alternative.should_receive(:increment_participation) experiment.should_receive(:winner).and_return nil diff --git a/split.gemspec b/split.gemspec index 141aa781..01360ccb 100644 --- a/split.gemspec +++ b/split.gemspec @@ -32,6 +32,4 @@ Gem::Specification.new do |s| s.add_development_dependency 'bundler', '~> 1.0' s.add_development_dependency 'rspec', '~> 2.12' s.add_development_dependency 'rack-test', '~> 0.6' - s.add_development_dependency 'pry' - s.add_development_dependency 'pry-debugger' end From c5babac5aca70309af247f2249f5fedd173d7713 Mon Sep 17 00:00:00 2001 From: Nathan Woodhull Date: Mon, 7 Jan 2013 13:25:06 -0500 Subject: [PATCH 15/15] a few more specs. --- spec/algorithms/weighted_sample_spec.rb | 5 +++++ spec/algorithms/whiplash_spec.rb | 7 ++++++- spec/trial_spec.rb | 21 ++++++++++++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/spec/algorithms/weighted_sample_spec.rb b/spec/algorithms/weighted_sample_spec.rb index 8407ac7a..5f3fc64c 100644 --- a/spec/algorithms/weighted_sample_spec.rb +++ b/spec/algorithms/weighted_sample_spec.rb @@ -1,6 +1,11 @@ require "spec_helper" describe Split::Algorithms::WeightedSample do + it "should return an alternative" do + experiment = Split::Experiment.find_or_create('link_color', {'blue' => 100}, {'red' => 0 }) + Split::Algorithms::WeightedSample.choose_alternative(experiment).class.should == Split::Alternative + end + it "should always return a heavily weighted option" do experiment = Split::Experiment.find_or_create('link_color', {'blue' => 100}, {'red' => 0 }) Split::Algorithms::WeightedSample.choose_alternative(experiment).name.should == 'blue' diff --git a/spec/algorithms/whiplash_spec.rb b/spec/algorithms/whiplash_spec.rb index fd43e4b7..09cb0d5c 100644 --- a/spec/algorithms/whiplash_spec.rb +++ b/spec/algorithms/whiplash_spec.rb @@ -1,7 +1,12 @@ require "spec_helper" describe Split::Algorithms::Whiplash do - + + it "should return an algorithm" do + experiment = Split::Experiment.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) + Split::Algorithms::Whiplash.choose_alternative(experiment).class.should == Split::Alternative + end + it "should return one of the results" do experiment = Split::Experiment.find_or_create('link_color', {'blue' => 1}, {'red' => 1 }) ['red', 'blue'].should include Split::Algorithms::Whiplash.choose_alternative(experiment).name diff --git a/spec/trial_spec.rb b/spec/trial_spec.rb index 917b4ba7..2fad43cb 100644 --- a/spec/trial_spec.rb +++ b/spec/trial_spec.rb @@ -17,7 +17,26 @@ trial.alternative.should == alternative end - it "should call select_alternative if nil" do + it "should populate alternative with a full alternative object after calling choose" do + experiment = Split::Experiment.new('basket_text', :alternative_names => ['basket', 'cart']) + experiment.save + trial = Split::Trial.new(:experiment => experiment) + trial.choose + trial.alternative.class.should == Split::Alternative + ['basket', 'cart'].should include(trial.alternative.name) + end + + it "should populate an alternative when only one option is offerred" do + experiment = Split::Experiment.new('basket_text', :alternative_names => ['basket']) + experiment.save + trial = Split::Trial.new(:experiment => experiment) + trial.choose + trial.alternative.class.should == Split::Alternative + trial.alternative.name.should == 'basket' + end + + + it "should choose from the available alternatives" do trial = Split::Trial.new(:experiment => experiment = mock('experiment')) experiment.should_receive(:next_alternative).and_return(alternative = mock('alternative')) alternative.should_receive(:increment_participation)