wam: reject non-positive input weights instead of returning NaN - #825
Open
arpitjain099 wants to merge 1 commit into
Open
wam: reject non-positive input weights instead of returning NaN#825arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
WeightedArithmeticMean.Score returned itemSum/itemCount, which is NaN when the total weight of the matched inputs is zero. Score returns a bare float64 and cannot signal an error, so the failure was silent. Validate inputs in wam.New: any input with a zero or negative weight is now rejected with an error. Zero weights make the weighted mean undefined (division by zero), and negative weights have no meaningful interpretation in a weighted average, matching the maintainer guidance on the issue. Existing shipped scorer configs use only positive weights, so this adds no behavior change for valid configurations. Fixes ossf#314 Signed-off-by: Arpit Jain <arpitjain099@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
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.
What
WeightedArithmeticMean.ScorecomputesitemSum / itemCount, whereitemCountis the running sum of the matched input weights. When that total weight is zero, the result ismath.NaN(). BecauseScorereturns a barefloat64, it has no way to signal the failure, so an undefined score propagates silently to the caller.This validates the weights up front in
wam.Newinstead. Any input with a zero or negative weight is rejected with an error:NaN).This follows @calebbrown's direction on the issue: "Non-positive weights don't make much sense, and should cause an error if supplied."
Newalready returns(algorithm.Algorithm, error), so this is the natural place to surface the problem, andScore's behavior for valid inputs is unchanged.Why this is safe for existing configs
All shipped scorer configs (
config/scorer/*.yml) use only positive weights (1, 2, 0.5), so valid configurations are unaffected. The new error only fires on a misconfigured zero or negative weight.Tests
Extended the existing table-driven test in
wam_test.go:Newreturns an error (previouslyScorereturnedNaN)Newreturns an error (previouslyScoresilently returned a wrong value)Before this change those first two cases produce
NaN/ a silently-wrong value; after, they return a clear error.go test ./internal/scorer/...passes, andgofmt/go vetare clean.Fixes #314