split icon indicating copy to clipboard operation
split copied to clipboard

ArgumentError: Paramters must be greater than zero. [sic]

Open lrworth opened this issue 4 years ago • 4 comments

Describe the bug In split-4.0.0.pre, the dashboard page fails with an exception due to splitrb passing invalid parameters to BetaDistribution.

To Reproduce I haven't got a small reproduction case, but it is obvious what is happening. Within Split::Experiment#calc_beta_params:

  1. conversions == 1.
  2. We have some alternative where alternative.participant_count == 0.
  3. Under these conditions:
  • alpha = 1 + conversions so alpha == 1 + 1 == 2.
  • beta = 1 + alternative.participant_count - conversions so beta == 1 + 0 - 1 == 0
  1. In #calc_simulated_conversion_rates, beta is passed to RubyStats::BetaDistribution.
  2. BetaDistribution#initialize requires beta > 0 and raises an exception.

Assuming there is no bug in the logic here, a good resolution might be to only call experiment.calc_winning_alternatives within dashboard/views/_experiment.erb if it will succeed.

lrworth avatar Mar 26 '21 00:03 lrworth

I get a very similar problem with split-3.4.1. Perhaps I'm using it incorrectly?

lrworth avatar Mar 26 '21 00:03 lrworth

Probably related to #563.

lrworth avatar Mar 26 '21 01:03 lrworth

Hi there, yeah, it's probably related to that. BetaDistribution requires > 0.

As the participation_count should not be less than the completed count. At first, the solution was to ensure the participation_count was also increased when someone forced that alternative. But then I reworked that on https://github.com/splitrb/split/pull/637 in order to not take admin into metrics. So, it should not be possible to get into the same problem on v4.

But that still leaves a problem where the counts on alternatives may already be wrong.

I can make a patch to avoid running the BetaDistribution in such situations and think about how I can warn users about this on the dashboard.

For now, you could alternative.participant_count = completion_count for the alternatives where the 'conversions' is greater than that. WDYT?

andrehjr avatar Mar 27 '21 15:03 andrehjr

Interesting. I’m not sure how I got my database into such a state but clearing Redis seems to have done the trick.

If it’s always the case that participation_count >= completion_count then I’d personally prefer some kind of error describing which “impossible” thing happened so that I could fix it. But as you seem to have made this situation far less likely, it might not be worth the effort. Perhaps a guard and error log surrounding the call to BetaDistribution will suffice? If so I’m happy to provide a PR.

lrworth avatar Mar 28 '21 04:03 lrworth