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

Modified documentation for SparseCategoricalCrossentropy #34343

Closed
wants to merge 1 commit into from

Conversation

td2014
Copy link
Contributor

@td2014 td2014 commented Nov 16, 2019

The documentation example for SparseCategoricalCrossentropy is mathematically inconsistent with a parallel example given for CategoricalCrossentropy and also is confusing. In particular the second element in the original SparseCategoricalCrossentropy example function call has component given by [.5, .89, .6] which is not normalized to be a probability (i.e. doesn't sum to 1.0). I modified it to be [.05, .89, .06] which does sum to 1.0 and also recomputed the loss to be 0.09458992, preserving all the precision from the function call. I hope this helps clarify things for users. Thanks.

The documentation example for SparseCategoricalCrossentropy is mathematically inconsistent with a parallel example given for CategoricalCrossentropy and also is confusing.  In particular the second element in the original SparseCategoricalCrossentropy example function call has component given by [.5, .89, .6] which is not normalized to be a probability (i.e. doesn't sum to 1.0).  I modified it to be [.05, .89, .06] which does sum to 1.0 and also recomputed the loss to be 0.09458992, preserving all the precision from the function call.  I hope this helps clarify things for users.
@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label Nov 16, 2019
@gbaned gbaned self-assigned this Nov 18, 2019
@gbaned gbaned added the comp:keras Keras related issues label Nov 18, 2019
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Nov 18, 2019
Copy link
Contributor

@stmugisha stmugisha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try putting the usage example in doctest format as in this guide below? https://www.tensorflow.org/community/contribute/docs_ref

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Nov 20, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 20, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 20, 2019
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Dec 3, 2019
@gbaned
Copy link
Contributor

gbaned commented Dec 6, 2019

@td2014 Can you please resolve conflicts? Thanks!

@gbaned gbaned added stat:awaiting response Status - Awaiting response from author and removed ready to pull PR ready for merge process labels Dec 6, 2019
@tensorflowbutler tensorflowbutler added the stale This label marks the issue/pr stale - to be closed automatically if no activity label Dec 21, 2019
@tensorflowbutler
Copy link
Member

It has been 14 days with no activity and the awaiting response label was assigned. Is this PR still valid? Assigning the stalled label. Please comment to reassure me that this is still being worked on.

@pavithrasv
Copy link
Member

Closing this PR as the docs have been updated.

@pavithrasv pavithrasv closed this Dec 21, 2019
PR Queue automation moved this from Approved by Reviewer to Closed/Rejected Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes comp:keras Keras related issues size:XS CL Change Size: Extra Small stale This label marks the issue/pr stale - to be closed automatically if no activity stat:awaiting response Status - Awaiting response from author
Projects
PR Queue
  
Closed/Rejected
Development

Successfully merging this pull request may close these issues.

None yet

7 participants