-
Notifications
You must be signed in to change notification settings - Fork 860
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
Ignore abstains in Scorer, change LabelModel default tie break policy #1450
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1450 +/- ##
=======================================
Coverage 97.58% 97.58%
=======================================
Files 55 55
Lines 2029 2029
Branches 334 334
=======================================
Hits 1980 1980
Misses 22 22
Partials 27 27
|
@@ -472,6 +472,11 @@ def score( | |||
>>> label_model.score(L, Y=np.array([1, 1, 1]), metrics=["f1"]) | |||
{'f1': 0.8} | |||
""" | |||
if tie_break_policy == "abstain": # pragma: no cover | |||
logging.warning( | |||
"Metrics calculated over datapoints with non-abstain labels only" |
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.
Nit: we've been using data points (2 words)
test/analysis/test_scorer.py
Outdated
# Test abstain=-1 for preds and gold | ||
abstain_preds = np.array([-1, -1, 1, 1, 0]) | ||
abstain_probs = np.array([0.5, 0.5, 0.9, 0.7, 0.4]) | ||
results = scorer.score(golds, abstain_preds, abstain_probs) |
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.
No need to pass in probs here. They're optional, and you're only calculating accuracy in this scorer, which just requires golds and preds.
@@ -209,6 +209,13 @@ def test_predict_proba(self): | |||
np.testing.assert_array_almost_equal(probs, true_probs) | |||
|
|||
def test_predict(self): | |||
L = np.array([[-1, 1, 0], [0, -1, 1], [1, 0, -1]]) |
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.
Can we add a simple comment here that this test is confirming that with 3 LFs that counteract one another results in tie votes and therefore abstains on all points?
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.
Changes lgtm! Ship it!
Description of proposed changes
score()
functionTest plan
predict()
andscore()
functions related to abstain defaultChecklist