Skip to content

[SPARK-58621][ML] Add descriptive messages to SummarizerBuffer metric accessor require checks - #57823

Open
uros-b wants to merge 1 commit into
apache:masterfrom
uros-b:error-summarizer-metrics
Open

[SPARK-58621][ML] Add descriptive messages to SummarizerBuffer metric accessor require checks#57823
uros-b wants to merge 1 commit into
apache:masterfrom
uros-b:error-summarizer-metrics

Conversation

@uros-b

@uros-b uros-b commented Aug 6, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Adds a short descriptive message to the nine bare require(requestedMetrics.contains(X)) guards in SummarizerBuffer, so that reading a metric that was not requested fails with a message naming the metric:

require(requestedMetrics.contains(Mean), "mean was not a requested metric.")

Why are the changes needed?

These guards currently produce a bare requirement failed, which does not indicate which metric was involved. The quoted names match the user-facing metric names accepted by Summarizer.metrics(...) per the allMetrics table, including numNonZeros (the metric string differs in casing from the numNonzeros accessor, so the message quotes the name a caller actually passes).

These are plain require calls rather than part of the structured error-condition framework, and the adjacent require(totalWeightSum > 0, ...) check in each of the same methods already carries a message, so this brings the metric guards in line with their immediate neighbours.

Does this PR introduce any user-facing change?

No. SummarizerBuffer is private[spark], and only the message text of an existing check is added.

How was this patch tested?

Conditions and exception types are unchanged, so existing tests continue to apply; the error-handling tests in SummarizerSuite assert only the exception type and do not match on message text.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8)

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for CI. @zhengruifeng Please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant