This repository was archived by the owner on Nov 17, 2023. It is now read-only.
[MXNET-171] Fix a bug that was causing training accuracy to be printed as nan sometimes #10437
Conversation
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
|
Output of finetune notebook before fix: Output of finetune notebook after fix: |
piiswrong
reviewed
Apr 6, 2018
python/mxnet/module/base_module.py
Outdated
| @@ -523,8 +524,9 @@ def fit(self, train_data, eval_data=None, eval_metric='acc', | |||
| monitor.toc_print() | |||
|
|
|||
| if batch_end_callback is not None: | |||
| arg_eval_metric = eval_metric if not end_of_batch else deepcopy(eval_metric) | |||
There was a problem hiding this comment.
This is too hacky and potentially very slow
There was a problem hiding this comment.
Deep copy happens maximum once an epoch. That adds negligible time to an epoch.
Hacky - yes. That's because we are trying to print metrics outside the loop which could get cleared inside the loop. Any solution that doesn't modify the existing behavior will be a little hacky.
There was a problem hiding this comment.
If you can think of a less hacky solution, please propose. If not, I think this is better than printing accuracy as nan in training log which creates bad user experience. A number of users have reported this.
overhead except for the batch in an epoch.
|
Deep copy is slow and can cause a lot of problems. We can't do this kind of hack in the main package |
Closed
rahul003
pushed a commit
to rahul003/mxnet
that referenced
this pull request
Jun 4, 2018
…d as nan sometimes (apache#10437) * Fix a bug that was causing training accuracy to be printed as nan sometimes. * Avoid the additional 'arg_eval_metric' variable. There should be no overhead except for the batch in an epoch. * Fix lint. * For the last batch, Capture metrics before callback and use it to print epoch metrics * Remove unused import
zheng-da
pushed a commit
to zheng-da/incubator-mxnet
that referenced
this pull request
Jun 28, 2018
…d as nan sometimes (apache#10437) * Fix a bug that was causing training accuracy to be printed as nan sometimes. * Avoid the additional 'arg_eval_metric' variable. There should be no overhead except for the batch in an epoch. * Fix lint. * For the last batch, Capture metrics before callback and use it to print epoch metrics * Remove unused import
This was referenced Aug 14, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
Fix a bug (issue #4253) that was causing training accuracy to be printed as nan sometimes
The "Epoch[x] Train-accuracy=xxx" line printed at the end of every epoch gives the impression that the accuracy metric is for the entire epoch. In reality, it is NOT.
For example if an epoch consists of 101 batches and we were printing metrics every 10 batch because callback was created that way, what will be printed as 'Train-accuracy=xxx' at the end of an epoch is actually just the accuracy from a single batch (101'st batch). Printing this as 'Epoch[x] Train-accuracy=xxx' is very misleading.
Ideally we should remove this misleading print statement. But then, I'm sure there is a lot of existing scripts out there that look for this statement. We can't remove this without breaking those scripts. Since this will be a breaking change, let's do it in a major version change.
For now, to avoid the nan error, we can avoid resetting the metrics when processing callback for the last batch. It will be reset at the beginning of the next epoch anyway.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.