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.

Sign up for GitHub

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

Jump to bottom

Matthew Fix #2406

Merged
merged 8 commits into from
Apr 2, 2021
Merged

Matthew Fix #2406

merged 8 commits into from
Apr 2, 2021

Conversation

jonpsy
Copy link
Contributor

@jonpsy jonpsy commented Mar 1, 2021

Description

Brief Description of the PR:

Fixes # (issue)
#2339

Type of change

  • Bug fix
  • New Tutorial
  • Updated or additional documentation
  • Additional Testing

Checklist:

  • [ x] I've properly formatted my code according to the guidelines
    • By running Black + Flake8
    • By running pre-commit hooks
  • This PR addresses an already submitted issue for TensorFlow Addons
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • This PR contains modifications to C++ custom-ops

How Has This Been Tested?

The issue created by the user is perfectly solved by this

If you're adding a bugfix or new feature please describe the tests that you ran to verify your changes:

jonpsy added 2 commits March 1, 2021 18:21
@jonpsy
initial changes
25ecbb4
@jonpsy
- doc test changed
1aadeab 
- multiclass works for OHE
- fix multiclass test in unit test
@jonpsy jonpsy requested a review from autoih as a code owner March 1, 2021 15:40
@boring-cyborg boring-cyborg bot added the metrics label Mar 1, 2021
@google-cla google-cla bot added the cla: yes label Mar 1, 2021
@bot-of-gabrieldemarmiesse
Copy link

@marload @autoih

You are owners of some files modified in this pull request.
Would you kindly review the changes whenever you have the time to?
Thank you very much.

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 1, 2021

This should work for one hot encoded label. I'll work on sparse labels as well but I wanted to gather some reviews before. Looking forward! :)

@bhack
Copy link
Contributor

bhack commented Mar 18, 2021

@ZeynepP Can you test this fix?

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 19, 2021

@bhack I've added test case which confirms that patch solves the issue. Would you mind reviewing this patch?

@bhack
Copy link
Contributor

bhack commented Mar 19, 2021 •
edited
Loading

Can we check numerically with https://scikit-learn.org/stable/modules/generated/sklearn.metrics.matthews_corrcoef.html ?

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 19, 2021 •
edited
Loading

Its in the test case :) or do you mean to say I should import sklearn and check their output values? I can do that.

@bhack
Copy link
Contributor

bhack commented Mar 19, 2021 •
edited
Loading

I should import sklearn and check their output values? I can do that.

Yes we already have some tests with sklearn in the repo. I think that you can check these.

@jonpsy
Copy link
Contributor Author

jonpsy commented Mar 20, 2021

Done :)

@bhack bhack merged commit e83e71c into tensorflow:master Apr 2, 2021
@jonpsy jonpsy deleted the matthew branch April 2, 2021 09:44
@lgeiger lgeiger mentioned this pull request Jun 2, 2021
Open
21 tasks
@lgeiger lgeiger mentioned this pull request Jun 2, 2021
Closed
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgeiger lgeiger lgeiger left review comments

@bhack bhack bhack approved these changes

@autoih autoih Awaiting requested review from autoih autoih is a code owner

Assignees
No one assigned
Labels
cla: yes metrics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jonpsy @bot-of-gabrieldemarmiesse @bhack @lgeiger