feat: Relative Effects and MDE for Ratio Metrics - #264
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
========================================
Coverage 95.17% 95.18%
========================================
Files 18 18
Lines 2094 2220 +126
========================================
+ Hits 1993 2113 +120
- Misses 101 107 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| transformer = DeltaMethodLiftTransformer(self.treatment_col) | ||
| transformer.fit( | ||
| mean_diff=mean_diff, | ||
| var_abs=treat_var + ctrl_var, |
There was a problem hiding this comment.
perhaps you can pass std_error instead of var,and you have it in this variable: standard_error
| "percent_lift": self._relative_lift_value, | ||
| "_se_relative_lift": self._se_relative_lift, | ||
| "pvalue": self.pvalues[self.treatment_col], | ||
| "conf_int": self.conf_int(0.05).loc[self.treatment_col], |
There was a problem hiding this comment.
shouldnt we use alpha in here?
…variance; update related tests - Changed the `DeltaMethodLiftTransformer` to accept standard error in the `fit` method instead of variance. - Updated the `experiment_analysis.py` to reflect this change in the transformer usage. - Refactored `relative_lift_transformer.py` to introduce a base class for lift transformers, consolidating shared functionality. - Modified the Jupyter notebook `relative_delta.ipynb` to align with the new transformer behavior and improve clarity in output formatting. - Added tests to ensure parity between OLS and DeltaMethodAnalysis results for relative effects.
|
|
||
| # Point estimates come from different estimators (unweighted vs weighted by | ||
| # scale) so a 5% relative tolerance is appropriate | ||
| assert ols_point == pytest.approx(delta_point, rel=0.05) |
There was a problem hiding this comment.
just curious. do you think we can get it closer?
There was a problem hiding this comment.
hmm did not think of that. maybe some simulations to check how close, or rather will check in some other sources
| "pvalue": self.pvalues[self.treatment_col], | ||
| "conf_int": self.conf_int(0.05).loc[self.treatment_col], | ||
| } | ||
| Solves A*m^2 + B*m + C = 0 for the smallest positive m satisfying |
There was a problem hiding this comment.
this is very new to me, could you add some reference or small proof?
There was a problem hiding this comment.
oki! I'll add references in the .py, but I'll add a more thorough view in a notebook
| "percent_lift": self._relative_lift_value, | ||
| "_se_relative_lift": self._se_relative_lift, | ||
| "pvalue": self.pvalues[self.treatment_col], | ||
| "conf_int": self.conf_int(0.05).loc[self.treatment_col], |
There was a problem hiding this comment.
let's change this 0.05 to alpha
| ) | ||
|
|
||
| assert transformer.bse["treatment"] >= se_naive | ||
| assert transformer.bse["treatment"] == pytest.approx(se_naive, rel=0.15) |
| "cluster_experiments.relative_lift_transformer.stats.norm.ppf", mock_ppf | ||
| ) | ||
|
|
||
| with pytest.raises(ValueError, match="invalid power equation"): |
There was a problem hiding this comment.
in which scenario do we not get an MDE?
There was a problem hiding this comment.
I'm adding scenarios hashtags comments, but be more specific on the types of errors where the MDE fails (ie, high variance, imcompatible variance with asked alpha,etc)
| return relative_lift, float(np.sqrt(var_relative)) | ||
|
|
||
| @staticmethod | ||
| def relative_mde( |
There was a problem hiding this comment.
after having a look I like this, but I think at some point it better bolngs in the power analysis class. Maybe we can merge this changing to internal method but later we move to NormalPowerAnalysis and use it to calculate the relative mde of ols
| def relative_mde( | |
| def _relative_mde( |
There was a problem hiding this comment.
but I don't see this method being called anyway, is this correct?
| return relative_lift, float(np.sqrt(var_relative)) | ||
|
|
||
| @staticmethod | ||
| def _relative_mde( |
There was a problem hiding this comment.
imo this is a responsibility of the PowerAnalysis class, I would move it in there
| standard_error = np.sqrt(treat_var + ctrl_var) | ||
|
|
||
| if self.relative_effect: | ||
| transformer = DeltaMethodLiftTransformer(self.treatment_col) |
There was a problem hiding this comment.
a bit confused in here, where is this method used? does this handle covariates?
There was a problem hiding this comment.
Used by [_get_mean_standard_error] (p-value/SE path) and analysis_standard_error_with_stats(power path).
For Covariates: yes. It calls [_get_group_statistics] which builds [thetas_dict] from [self.covariates] and applies CUPED variance reduction inside [_get_group_mean_and_variance] So relative effects respect covariates.
|
Hey @david26694 ! Summary of the main changes:
One decision I’d like your opinion on before finalizing: |
|
hey @david26694 , to not clog the PR too much, as a follow up I propose the following notebook explaining the %MDE for ratio metrics (with Claude) Steps
Relevant files
Verification
|
feat: Relative Effects and MDE for Ratio Metrics
Summary
This PR adds proper relative (percent) lift estimation and minimum detectable effect (MDE) calculation for ratio metrics using an outer delta method, instead of relying on naive scaling by the control mean.
Problem
For ratio metrics such as conversion rate or revenue per user:
What Changed
Impact
Users can now estimate relative effects for ratio metrics with uncertainty propagation that is statistically consistent with delta-method assumptions, and compute a principled relative MDE via the quadratic formulation.
Notes
This keeps backward compatibility while introducing the new transformer API and helper functions for direct use in custom workflows.