mozilla / experimenter

A web application for managing user experiments for Mozilla Firefox.
https://experimenter.services.mozilla.com
Mozilla Public License 2.0
118 stars 183 forks source link

DataPoint.has_bounds implementation correctness #11605

Open brennie opened 1 day ago

brennie commented 1 day ago

The DataPoint.has_bounds implementation is:

    def has_bounds(self) -> bool:
        return self.lower and self.upper

which will report false for self.lower = self.upper = 0.

Updating this check to self.lower is not None and self.upper is not None is probably more correct but breaks unit tests. (The log is too long to attach as a comment to this issue.)

This happens because we end up in compute_signficance from here because has_bounds() is now true and the compute_signficance method doesnt expect lower = upper = 0

┆Issue is synchronized with this Jira Task

brennie commented 1 day ago

Test failure log: https://www.pastery.net/nbmzwm/

brennie commented 1 day ago

relevant section:

{'absolute': {'all': [],
E                                                                                                                         'first': {}},
E                                                                                                            'difference': {'control': {'all': [],
E                                                                                                                                       'first': {}},
E                                                                                                                           'variant': {'all': [{'lower': 0.0,
E                                                                                                                                                'point': 0.0,
E                                                                                                                                                'upper': 0.0},
E                                                                                                                                               {'lower': 0.0,
E                                                                                                                                                'point': 0.0,
E                                                                                                                                                'upper': 0.0}],
E                                                                                                                                       'first': {'lower': 0.0,
E                                                                                                                                                 'point': 0.0,
E                                                                                                                                                 'upper': 0.0}}},
E                                                                                                            'relative_uplift': {'control': {'all': [],
E                                                                                                                                            'first': {}},
E                                                                                                                                'variant': {'all': [],
E                                                                                                                                            'first': {}}},
E                                                                                                            'significance': {'control': {'overall': {},
E                                                                                                                                         'weekly': {}},
E   -                                                                                                                         'variant': {'overall': {'1': 'negative'},
E   ?                                                                                                                                                 ---------------
E   
E   +                                                                                                                         'variant': {'overall': {},
E                                                                                                                                         'weekly': {}}}},