-
Notifications
You must be signed in to change notification settings - Fork 393
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
Add multiclassification topk and confmatrix metrics to model insights serialization format #537
Conversation
Codecov Report
@@ Coverage Diff @@
## master #537 +/- ##
==========================================
+ Coverage 78.80% 86.78% +7.97%
==========================================
Files 347 347
Lines 12019 12025 +6
Branches 394 384 -10
==========================================
+ Hits 9472 10436 +964
+ Misses 2547 1589 -958
Continue to review full report at Codecov.
|
@@ -232,8 +232,8 @@ class RecordInsightsLOCOTest extends FunSpec with TestSparkContext with RecordIn | |||
info("Each feature vector should only have either three or four non-zero entries. One each from country and " + | |||
"picklist, while currency can have either two (if it's null the currency column will be filled with the mean)" + | |||
" or just one if it's not null.") | |||
it("should pick between 1 and 4 of the features") { | |||
all(parsed.map(_.size)) should (be >= 1 and be <= 4) | |||
it("should pick between 0 and 4 of the features") { |
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.
why did this change?
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.
@tovbinm this test is quite flaky. In one of the failed runs, the value was 0 and not between 1 and 4.
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.
@leahmcguire @Jauntbox do you have any ideas?
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.
There's still an open PR to address this: #423
We could use a better test, but for now we can start by getting rid of the flakiness.
…est to check serialization/deseriazalition of multiclassification metrics
@crupley @nicodv include someone from the team to prioritize review this PR, changes in this PR will fix test that's currently blocking https://github.salesforceiq.com/einstein/einstein1/pull/4010 for bumping version in automl |
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.
LGTM
No description provided.