[Feature request] Allow `Scientist::Experiment` classes to *not* be default Scientist experiment class

[danielvdao]

I'm wondering what the original inspiration for making any class that includes the module the default experiment, code here. Is it possible to create a class that includes the Scientist::Experiment module and not make it the default scientist experiment?

Looking here it doesn't seem like it is possible.

From docs as well:

When Scientist::Experiment is included in a class, it automatically sets it as the default implementation via Scientist::Experiment.set_default. This set_default call is is skipped if you include Scientist::Experiment in a module.

The reason I ask is because recently we ran into an issue with our Rails app:

1. We had a class that was running Scientist via include Scientist with the default experiment, but was not publishing anything, oops.
2. We created a new class for that included Scientist::Experiment, but since our tests don't eager load our classes, it was a flaky test and not surfaced during our build.
3. When running in production, we eager loaded and overrode the default experiment and then surfaced issues.

So a few questions I was wondering:

1. Why default experiment, this seems a little aggressive IMO?
2. Can we actually find a way to either make default the normal behavior and have a way to pass a flag to override it?
3. Or is it that the way we were using it was wrong, and that the intention is to actually just include Scientist and override the default methods for that class? If so, the only reason I held back from doing so is better separation of responsibilities. It was going to look a bit messy to clog up one class's specs with experiment specs and I'd rather separate the two easily 🤔
4. Relating to point 3, is it the case that by the current implementation we can't be running multiple experiments at once?

I looked into it a bit myself, but it doesn't seem like there is a reasonably clean way. Although I think we can add a class method (something like):

class Foo
  include Scientist::Experiment

  default_scientist_experiment(false) # new
end

What do y'all think? If so - I can try to take a stab at it.

[danielvdao]

So I went ahead and tried to take a stab at it - but I realized that there is something eerily similar here! (code link)

That being said - I can do something like Scientist::Experiment.set_default_experiment(false) when the class is included.. Is the better behavior to do something like:

class Foo
  include Scientist::Experiment

  Scientist::Experiment.default_experiment(Scientist::Default)
end

Though that kind of exposes class internals (e.g. if you ever want to switch clients off from using Scientist::Default, this would be a breaking change, so maybe the original suggestion could work 🤔 ). What do y'all think?

[danielvdao]

Linked PR - https://github.com/github/scientist/pull/163

[danielvdao]

Hi @zerowidth I noticed that you recently committed to the repo. I'm wondering if you could take a look at the issue and give the PR a review? If it doesn't make sense, or if it's not valid, I'm happy to close it. But it's been sitting for 3 months open and I'd like to try and address anything / bring it to a close if possible.

[benoittgt]

I think the "creating class make it default" is a weird pattern.

It should be clearer. Like a Scientist initializer that specificy the custom default experiment.

Related : https://github.com/github/scientist/issues/92

[danielvdao]

Hey @benoittgt I'd love to merge the PR I had set up or keep pushing through it. Last I tagged someone, I wasn't sure who / didn't get any comments so I wasn't sure what to do.

[madejejej]

Hey guys, any update here? IMO the most flexible approach would be being able to pass an Experiment class for any invocation of the Scientist.science method. Some ideas:

# pass the class in `opts`:
Scientist.science 'my-experiment', experiment_class: MyExperiment do |e|
end

# perhaps we could just pass an instance of Experiment 
# OR the experiment name to use the default Experiment implementation?

# Use the passed instance
Scientist.science MyExperiment.new('my-experiment') do |e|
end

# Use the default implementation
Scientist.science  'my-experiment' do |e|
end

[omkarmoghe]

@madejejej Found this issue because we're also facing some friction with the flexibility of the science method. I agree with the idea of allowing you to just pass an experiment class into the call.

One thing I wanted to call out in your example was that if you're instantiating the experiment yourself like MyExperiment.new('my-experiment'), then you don't really need the science call at all IMO, you already have an instance you can call use, try, etc. on.

A feature that could be cool is being able to implement the control and candidate directly in the experiment class so you don't have to call those methods each time 🤔

class MyExperiment
 include Scientist::Experiment

  def initialize(opts = {})
    super("my-experiment")
    @foo = opts[:foo]
  end

  def enabled?
  end

  def control
  end

  # I think this is an array in the library implementation, but just an example
  def candidate
  end 
end

and then elsewhere you can just run

MyExperiment.new(foo: "bar").run

[alexpech12]

Hey folks, I'm also unsure how best to structure things so that multiple experiments can be run at a time, and I find the "make class default when Scientist::Experiment included" a strange pattern

I agree with @madejejej's suggestion to pass an Experiment class to Scientist.science for the best flexibility.

I'd also suggest that classes that include Scientist::Experiment should not make themselves the 'default experiment' by default. I'd prefer it if I could set this explicitly in an initialiser

Scientist::Experiment.set_default_experiment(Scientist::DefaultExperiment)

It could also be that I'm not understanding the purpose and usage of Scientist::Experiment – and if that's the case I'd love to see some more concrete examples in the README, particularly with suggestions of how to structure things in a Rails project

[omkarmoghe]

@alexpech12 So I ended up taking a stab at an alternate implementation of a Ruby experimentation lib that's heavily inspired by Scientist, but addresses the concerns you and others in this thread have raised: https://github.com/omkarmoghe/lab_coat. I think it "plays nice" with Rails applications, which is my use-case as well. Just mentioning it here in case you or anyone else in this thread finds it useful. Open to feedback as well.

[alexpech12]

Thanks @omkarmoghe, I'll check it out! I'm also exploring an alternative where I avoid using Scientist.science and always create an explicit instance of an Experiment class, which seems to serve the same purpose.

[danielvdao]

Hey y'all - I mentioned earlier in my comment that I'd love to advance or work on this in any way possible, but it doesn't seem like there's much traction on this. I'm not sure how to get maintainer eyes on it (and tagged one earlier as well), but am always happy to keep pushing my current iteration / branch forward.

The PR I linked https://github.com/github/scientist/pull/163 was my first stab - but definitely not meant to be the final solution.

[danielvdao]

cc @zerowidth @ekroon thoughts?

[ekroon]

@danielvdao If you do not want to override the default experiment class and you want to just run an experiment without calling science, you can use:

Scientist.run "experiment-name" do |e|
  # do the usual things normally run in a "science" call. 
end

[ozydingo]

@ekroon this still uses an implicit, single, default experiment class. This doesn't allow flexibility to, for example, run experiments that have different publish, raise, and enabled behaviors.

I suggest @omkarmoghe's suggestion is the cleanest path forward. Keep the default_experiment behavior, but make it equivalent to simply call run on an experiment instance. This allows app developers full control over what experiment setup to use in a given part of code, allowing a more OOP mental model instead of modeling a single, global entrypoint.

[ekroon]

@ozydingo If you want to publish different for a specific class you could do something like is done here: https://github.com/github/scientist/blob/a25a0d238b759dd70367c27a4aad683e188a1e5c/test/scientist/experiment_test.rb#L6

class MyExperiment
  include Scientist::Experiment
  Scientist::Experiment.set_default(nil)

  # Implement required methods
end

@ex = MyExperiment.new
@ex.try { ... }
@ex.run