Skip to content
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

Divide by Zero in Weighted Arithmetic Mean Algorithm #314

Open
nathannaveen opened this issue Jan 30, 2023 · 5 comments
Open

Divide by Zero in Weighted Arithmetic Mean Algorithm #314

nathannaveen opened this issue Jan 30, 2023 · 5 comments

Comments

@nathannaveen
Copy link
Contributor

The Weighted Arithmetic Mean algorithm in the wam package currently returns math.NaN() when totalWeight is zero.

Is this intended behavior or should the algorithm return an error in this scenario instead of math.NaN()?
In other scenarios, returning math.NaN() may be fine, but when the total weight of the inputs is zero, it is better to return an error so that the user of the algorithm is aware that the calculation failed and can take appropriate action.

@nathannaveen
Copy link
Contributor Author

cc @calebbrown 👀

@jailby
Copy link

jailby commented Feb 16, 2023

I think that to emulate a negative weight you need to use the smaller_is_better attribute in the signal config like the updated_since signal:
https://github.com/ossf/criticality_score/blob/main/config/scorer/original_pike.yml#L27-L30

I think that's the way to handle negative signal.

@nathannaveen
Copy link
Contributor Author

@jailby I agree that smaller_is_better should be set when we have a negative weight, but what about a 0 weight? If we have totalWeight equal zero then we get math.NaN().

@calebbrown
Copy link
Contributor

Non-positive weights don't make much sense, and should cause an error if supplied.

  • zero weights are meaningless - it implies that the particular field should be ignored, but it could just be removed
  • negative weights are ambiguous - does it mean "smaller is better" or "the presence of this field should have a negative impact on the final score"? Algorithmically for a WAM a negative weight makes the total weight smaller, and cause the fields contribution to the total score to be negated, which is confusing.

Other ways of mutating the fields to make them work well in a aggregation function (like WAM) may be necessary (e.g. inverse, diff with another field), but that should be done separately.

@nathannaveen
Copy link
Contributor Author

@calebbrown So, should we create errors for zero and negative weights?

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

No branches or pull requests

3 participants