-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(suspect flags): Include filtered flag in output #95007
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #95007 +/- ##
===========================================
+ Coverage 38.17% 87.82% +49.65%
===========================================
Files 9858 10436 +578
Lines 556058 604299 +48241
Branches 23550 23550
===========================================
+ Hits 212265 530718 +318453
+ Misses 343426 73214 -270212
Partials 367 367 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Box-Cox Lambda Calculation Inconsistency
The boxcox_transform
function calculates the optimal lambda parameter using the original values
via _boxcox_normmax
, even when it internally shifts non-positive values
to shifted_values
for the actual transformation. This leads to an inconsistency where the optimal lambda is determined for a different dataset than what is ultimately transformed, potentially yielding suboptimal results. Furthermore, _boxcox_normmax
duplicates the shifting logic, which is inefficient and brittle.
src/sentry/seer/math.py#L109-L129
sentry/src/sentry/seer/math.py
Lines 109 to 129 in aa96455
shifted_values = values | |
if lambda_param is not None: | |
if lambda_param == 0.0: | |
transformed = [math.log(max(v, 1e-10)) for v in shifted_values] | |
else: | |
transformed = [ | |
(pow(max(v, 1e-10), lambda_param) - 1) / lambda_param for v in shifted_values | |
] | |
return transformed, lambda_param | |
optimal_lambda = _boxcox_normmax(values) | |
if optimal_lambda == 0.0: | |
transformed = [math.log(max(v, 1e-10)) for v in shifted_values] | |
else: | |
transformed = [ | |
(pow(max(v, 1e-10), optimal_lambda) - 1) / optimal_lambda for v in shifted_values | |
] | |
return transformed, optimal_lambda |
Was this report helpful? Give feedback by reacting with 👍 or 👎
src/sentry/seer/math.py
Outdated
] | ||
return transformed, lambda_param | ||
|
||
optimal_lambda = _boxcox_normmax(values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be _boxcox_normmax(shifted_values)
?
src/sentry/seer/math.py
Outdated
|
||
optimal_lambda = _boxcox_normmax(values) | ||
|
||
if optimal_lambda == 0.0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor one - looks like you can avoid code duplication by first initializing lambda_param as lambda_param = _boxcox_normmax(values) if lambda_param is not None else lambda_param
.
TODO:
Heuristic + RRF
orRRF