fix: Exclude non-finite values from numeric monitoring metrics - #6782
Open
piyush182004 wants to merge 1 commit into
Open
fix: Exclude non-finite values from numeric monitoring metrics#6782piyush182004 wants to merge 1 commit into
piyush182004 wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
compute_numeric() passed its values straight to np.histogram, which raises
on NaN/+-Inf:
ValueError: autodetected range of [0.05, inf] is not finite
Non-finite values arise from ordinary feature engineering - a click-through
rate whose denominator is zero, a log of zero, or missing values encoded as
NaN. compute_all() has no per-column error handling, so a single such value
discarded the metrics for the entire feature view, including columns that
were perfectly well-formed.
The failure was silent from the caller's perspective: the monitoring API
still returned HTTP 200 with status "completed" and computed_features 0, so
the UI showed no error - the feature view was simply absent from the
Monitoring page.
Filter non-finite values once, after nulls are dropped, so every statistic
is computed over finite data, and apply the existing _safe_float/opt_float
helpers to min_val, max_val and the quantiles (previously only mean and
stddev were guarded, letting Inf leak into the stored metrics).
row_count/null_count continue to describe the raw data.
The same unguarded np.histogram call is duplicated in the Dask offline
store's _dask_compute_numeric_metrics, and is fixed there too.
Signed-off-by: Piyush Mondal <piyushmondal182004@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ISSUE FIX:- #6783
What this PR does / why we need it:
MetricsCalculator.compute_numeric() passed values straight to
np.histogram(), which raises on NaN/+-Inf:
Non-finite values arise from ordinary feature engineering - a
click-through rate whose denominator is zero, a log of zero, or missing
values encoded as NaN. compute_all() has no per-column error handling,
so a single such value discarded the metrics for the entire feature view,
including columns that were perfectly well-formed.
The failure was silent from the caller's perspective: the monitoring API
still returned HTTP 200 with "status": "completed" and
"computed_features": 0, so the UI showed no error - the feature view was
simply absent from the Monitoring page.
This PR filters non-finite values once, after nulls are dropped, so every
statistic is computed over finite data, and applies the existing
_safe_float/opt_float helpers to min_val, max_val and the
quantiles (previously only mean and stddev were guarded, letting Inf
leak into stored metrics). row_count/null_count continue to describe
the raw data.
The same unguarded np.histogram call is duplicated in the Dask offline
store's _dask_compute_numeric_metrics, and is fixed there too.
Verification
Before: a feature view containing one inf value produced zero metrics
for all of its features. After: full metrics are produced, computed
correctly over the finite values (verified against an identical feature
view with no inf, which behaved correctly on both sides).
Full unit suite: 315 passed, 24 skipped. 6 new regression tests added.
Which issue(s) this PR fixes:
Fixes #<ISSUE_NUMBER>
Checks
Testing Strategy