Skip to content

Loss calculated incorrectly in networks_seq2seq_nmt.ipynb #2637

Open
@martingoodson

Description

@martingoodson

System information

  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): N/A
  • TensorFlow version and how it was installed (source or binary): N/A
  • TensorFlow-Addons version and how it was installed (source or binary): N/A
  • Python version: N/A
  • Is GPU used? (yes/no): N/A

Describe the bug
This bug is in https://colab.research.google.com/github/tensorflow/addons/blob/master/docs/tutorials/networks_seq2seq_nmt.ipynb

The loss function is not calculated properly. The mean should only be calculated over non-masked elements. This line should be replaced:

loss = tf.reduce_mean(loss)

with this:

loss = tf.math.reduce_sum(loss) / tf.math.reduce_sum(mask)

This now gives the same results as keras.metrics.SparseCategoricalCrossentropy(from_logits=True), as expected.

def loss_function(real, pred):
  # real shape = (BATCH_SIZE, max_length_output)
  # pred shape = (BATCH_SIZE, max_length_output, tar_vocab_size )
  cross_entropy = tf.keras.losses.SparseCategoricalCrossentropy(from_logits=True, reduction='none')
  loss = cross_entropy(y_true=real, y_pred=pred)
  mask = tf.logical_not(tf.math.equal(real,0))   #output 0 for y=0 else output 1
  mask = tf.cast(mask, dtype=loss.dtype)  
  loss = mask* loss
  loss = tf.reduce_mean(loss)
  return loss  

Code to reproduce the issue

Provide a reproducible test case that is the bare minimum necessary to generate the problem.

Other info / logs

Include any logs or source code that would be helpful to diagnose the problem. If including tracebacks, please include the full traceback. Large logs and files should be attached.

Activity

guillaumekln

guillaumekln commented on Dec 28, 2021

@guillaumekln
Contributor

Yes, the loss is not correctly reduced. Could you send a PR with your change?

MrinalTyagi

MrinalTyagi commented on Jan 28, 2022

@MrinalTyagi

@guillaumekln I would like to contribute to this if no one is working on it.

martingoodson

martingoodson commented on Jan 28, 2022

@martingoodson
Author
MrinalTyagi

MrinalTyagi commented on Jan 28, 2022

@MrinalTyagi

I will submit this pull request shortly. I've been on holiday.

-- Martin Goodson @martingoodson - -
On Fri, Jan 28, 2022 at 5:58 AM MrinalTyagi @.> wrote: @guillaumekln https://github.com/guillaumekln I would like to contribute to this if no one is working on it. — Reply to this email directly, view it on GitHub <#2637 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANXGHLX74PYS7GWYICJMADUYIWATANCNFSM5KVRSXNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you authored the thread.Message ID: @.>

sorry. thought it was available for contribution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @martingoodson@guillaumekln@MrinalTyagi

        Issue actions

          Loss calculated incorrectly in networks_seq2seq_nmt.ipynb · Issue #2637 · tensorflow/addons