BugFix of "AttributeError: 'ProgbarLogger' object has no attribute 'target'" (or 'log_values') error (#12898) (#12893) (#8944) #12899
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
…arget'" (or 'log_values') error (keras-team#12898) (keras-team#12893) (keras-team#8944) This Change set is fixing two missing attribute bugs: * AttributeError: 'ProgbarLogger' object has no attribute 'target' keras-team#12898 Abstract reproduction scenario is provided in ticket * AttributeError: 'ProgbarLogger' object has no attribute 'log_values' keras-team#3657 * AttributeError: 'ProgbarLogger' object has no attribute 'log_values' keras-team#8944 (dup) Related changes: * Cases of regression are covered by tests. * Some potential bugs with same nature are prevented and covered by manifestation checks. * run with empty data array (but having valid shape) is now handled properly and yielding related warnings on callback and training routine level without execution fail Note: Changes that affect `ProgbarLogger` should be aware of following things: * proper target initialisation is requiring two attributes: `params` and `use_steps` to be defined * `use_steps` is guaranteed attribute the is set in the constructor (but could be altered after object instantiation. It's currently safe condition. * class `params` attribute could be altered between initialisation and training start. And current logic is made to be aware of this * we don't have `params` initialisation in constructor, this attribute will be assigned on call of `set_params` of base class somewhere on caller level (no strict guarantees :( ) * `seen` attribute is working in pair with `target` during `log_values` initialisation and their initialisation should be under the equal condition, currently thats not true * `if self.seen < self.target` condition is being checked whenever verbose mode value so both of them should be initialised without any conditions * `if self.seen < self.target` is checking for training iteration being not finished but in case of degenerate case with zero length it will not be called and `log_values` will stay not initialised but i don't see any explicit logic preventing using it on exit from 0-length training cycle and potentially it is the bug of some kind that is prevented on caller logic level * `progbar` attribute initialisation is definitely related to output verbosity (log values accumulation are not) and should be left under verbosity condition
|
Guys, any assistance with landing, please? |
fchollet
reviewed
Jun 23, 2019
keras/callbacks.py
Outdated
| @@ -567,19 +573,22 @@ def __init__(self, count_mode='samples', | |||
| def on_train_begin(self, logs=None): | |||
| self.verbose = self.params['verbose'] | |||
| self.epochs = self.params['epochs'] | |||
| self.log_values = [] | |||
| if self.target is 0: | |||
There was a problem hiding this comment.
You can remove the warning, since it will not be informative for the user (it assumes too much context)
keras/callbacks.py
Outdated
| log_values = None | ||
| target = 0 | ||
| seen = 0 | ||
| progbar = None |
There was a problem hiding this comment.
Instance attributes should never be class attributes. They should be defined in init. You may not realize it, but everything you define at the class level (like here) is shared across instances.
* Remove unnecessary warning about empty input. keras-team#12898 keras-team#12893 keras-team#8944 keras-team#12899
|
Sorry if I'm stepping on any toes by jumping in, but it looks like the two failed tests on Travis should pass if you rebase this on top of the current Keras master. Specifically, 3e77ab6 seems to do the trick. |
|
Closing outdated PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This Change set is fixing two missing attribute bugs in
callback.ProgbarLoggerclassAbstract reproduction scenario is provided in ticket
Related changes:
and yielding related warnings on callback and training routine level without execution fail
Note:
Changes that affect
ProgbarLoggershould be aware of following things:paramsanduse_stepsto be defineduse_stepsis guaranteed attribute the is set in the constructor (but could be altered after object instantiation. It's currently safe condition.paramsattribute could be altered between initialisation and training start. And current logic is made to be aware of thisparamsinitialisation in constructor, this attribute will be assigned on call ofset_paramsof base class somewhere on caller level (no strict guarantees :( )seenattribute is working in pair withtargetduringlog_valuesinitialisation and their initialisation should be under the equal condition, currently thats not trueif self.seen < self.targetcondition is being checked whenever verbose mode value so both of them should be initialised without any conditionsif self.seen < self.targetis checking for training iteration being not finished but in case of degenerate case with zero length it will not be called andlog_valueswill stay not initialised but i don't see any explicit logic preventing using it on exit from 0-length training cycle and potentially it is the bug of some kind that is prevented on caller logic levelprogbarattribute initialisation is definitely related to output verbosity (log values accumulation are not) and should be left under verbosity conditionSummary
Related Issues
PR Overview