Catch class balance errors and test L matrix edge cases #1449
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1449 +/- ##
==========================================
+ Coverage 97.55% 97.55% +<.01%
==========================================
Files 55 55
Lines 2002 2008 +6
Branches 328 331 +3
==========================================
+ Hits 1953 1959 +6
Misses 22 22
Partials 27 27
|
| with self.assertRaisesRegex(ValueError, "L_train should have at least 3"): | ||
| label_model.fit(L, n_epochs=1) | ||
|
|
||
| def test_mv_default(self): |
There was a problem hiding this comment.
what change is this testing? do we actually add mv as a default anywhere here?
There was a problem hiding this comment.
it's checking for degenerate L matrices (aka low overlap and no conflicts) that the label model should internally default to the predictions MV would assign. it doesn't explicitly call majority vote anywhere
| label_model._set_class_balance(class_balance=class_balance, Y_dev=Y_dev) | ||
|
|
||
| Y_dev_one_class = np.array([0, 0, 0]) | ||
| with self.assertRaisesRegex(ValueError, "Y_dev has"): |
There was a problem hiding this comment.
nit: change substring to "Does not match LabelModel cardinality"
| ) | ||
| if len(self.p) != self.cardinality: | ||
| raise ValueError( | ||
| f"Y_dev has {len(self.p)} class(es). Does not match LabelModel cardinality {self.cardinality}." |
There was a problem hiding this comment.
this could also be because class_balance is the wrong size
There was a problem hiding this comment.
this error is also confusing for the Y_dev case. what's happening is that we're saying some of the class priors are 0 (by virtue of not being in Y_dev) which is the same as the above ValueError. rec: we raise a separate message under each of the if/else block (under if class_balance is not None: if it's the wrong size, then something likeClass balance prior is 0 in the other two cases.
Description of proposed changes
Related issue(s)
None
Test plan
Checklist