Skip to content

Conversation

@woodhull
Copy link
Contributor

For us, we'd like to use the default weighted sample algorithm for some experiments, but a bandit approach for others.

This pull request abstracts the algorithm used to choose alternatives in an experiment into a pluggable module.

There is a global configuration option for which algorithm to use, but it can be overridden on a per-experiment basis.

@andrew
Copy link
Member

andrew commented Jan 1, 2013

Looks great, would you be able to add some documentation and example config code to the readme?

…and configuration vs. redis storage. What a mess. Also tries to refactor helper.rb
@woodhull
Copy link
Contributor Author

woodhull commented Jan 3, 2013

So, it turns out that you'd actually like to persist the algorithm being used on a per experiment basis. This would let you use a bandit algorithm for some experiments, but a traditional split experiment for others.

This started me down a rabbit hole.

Issues I'm trying to work on:

  • Experiments can either come from a configuration file, or from redis -- but this was being handled very inconsistently in the code base.
  • Metrics were hacked into the helper module, rather than existing as a reusable object. It was only possible to define metrics inside the config file. It was not possible to use metrics for experiments that were loaded from redis rather than the config file.
  • Alternatives when stored in redis do not persist the set of weights associated with them.
  • Experiments should persist their configuration options on a per experiment basis. (resettable, algorithm, etc.)

In short this is not yet ready to merge, but is a work in progress you should be aware of.

Let me know if this seems like an insane direction!

@andrew
Copy link
Member

andrew commented Jan 4, 2013

No it sounds reasonable, ping me when it's ready!

@woodhull
Copy link
Contributor Author

woodhull commented Jan 4, 2013

I think this is ready to merge, can you take a peek?

@andrew
Copy link
Member

andrew commented Jan 4, 2013

Ok, there are a lot of changes in there, including test changes, which makes me wonder if it will have broken any backwards compatibility with previous versions that I'd like to check before merging.

Maybe @philnash and @buddhamagnet can give it once over as well?

@woodhull
Copy link
Contributor Author

woodhull commented Jan 4, 2013

Yes, as I indicated, it got away from me a bit.

I do not think that the semantics of any of the specs have changed, and I tried to not remove any assertions. Things have moved around a bit however.

@buddhamagnet
Copy link
Contributor

Hey all. My MBP is being repaired at the moment so won't have a chance to look for a few days. Will do ASAP and thanks for the endeavour @woodhull!

andrew added a commit that referenced this pull request Jan 12, 2013
@andrew andrew merged commit 798b4ef into splitrb:master Jan 12, 2013
@woodhull
Copy link
Contributor Author

Just making sure that history records that my friend @aaronsw is missed even here. Whiplash, the alternate bandit algorithm implementation in this pull request is derived from the VictoryKit and Whiplash projects he had been working on until he passed away last night. RIP.

@iangreenleaf
Copy link
Contributor

❤️

iangreenleaf added a commit to iangreenleaf/split that referenced this pull request Jan 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants