You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR fix an issue that caused metrics obtained with model.fit() being much lower than the ones obtained with model.evaluate() when Keras Masking is used.
This bug was observed when comparing training and evaluation metrics of a Transformer example (as described in #961 ), which makes usage of Keras Masking (SequenceMaskRandom, SequenceMaskLast) to select items of the sequence for training / eval.
Implementation Details 🚧
After investigation, I found out that the issue was been caused by the @tf.function decorator we had in model.train_compute_metrics(). After replacing a condition inside that function by tf.cond(), it was possible to remove the @tf.function decorator and fix the error when using Keras Masking (i.e., setting predictions._keras_mask).
Testing Details 🔍
Included tests in graph mode for test_train_metrics_steps, to double check that the logic inside model.train_compute_metrics() that skips steps for computing metrics continue to working in eager and graph mode.
gabrielspmoreira
changed the title
Fixes lower train metrics when using Keras Masking
Fixes lower train metrics when using Keras Masking (SequenceMaskRandom, SequenceMaskLast)
Feb 14, 2023
The decorator was added to fix that dataloader issue. There is unsuitability with list columns in the dataloader and adding the decorator fixed it. Do we still have issues with metrics if we use both tf.cond and tf.function?
The decorator was added to fix that dataloader issue. There is unsuitability with list columns in the dataloader and adding the decorator fixed it. Do we still have issues with metrics if we use both tf.cond and tf.function?
Hi Edward. I remember you have added some @tf.function decorator to deal with list features.
For this fix, I removed the @tf.function only from the train_compute_metrics(), which according to git blame was added by myself a while ago to be able to compute train metrics each N steps, in order to speed up training. So I think that @tf.function decorator is not related to you additions to fix the dataloader issue.
…m, SequenceMaskLast) (#983)
* Removed @tf.function from train_compute_metrics, as it is not needed and is causing a lower than real accuracy in model.fit() when using preds._keras_mask
* Turning if condition into tf.cond to remove tf.function decorator
* Making the should_compute_train_metrics_for_batch variable True by default
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
gabrielspmoreira
changed the title
Fixes #961
Goals ⚽
This PR fix an issue that caused metrics obtained with
model.fit()being much lower than the ones obtained withmodel.evaluate()when Keras Masking is used.This bug was observed when comparing training and evaluation metrics of a Transformer example (as described in #961 ), which makes usage of Keras Masking (SequenceMaskRandom, SequenceMaskLast) to select items of the sequence for training / eval.
Implementation Details 🚧
@tf.functiondecorator we had inmodel.train_compute_metrics(). After replacing a condition inside that function bytf.cond(), it was possible to remove the@tf.functiondecorator and fix the error when using Keras Masking (i.e., settingpredictions._keras_mask).Testing Details 🔍
test_train_metrics_steps, to double check that the logic insidemodel.train_compute_metrics()that skips steps for computing metrics continue to working in eager and graph mode.