Fix bug that could lead to duplicate params #783
Merged
Conversation
This file contains hidden or 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
When a net's module references another of that net's modules, the former will yield the latter's parameters. Therefore, when all parameters are collected, the latter's parameters appear twice. This bugfix consists of remembering all yielded parameters in a set and not yielding those that were already encountered. This bug was introduced very recently (751) and should occur very rarely, since modules don't typically reference each other.
thomasjpfan
reviewed
Jun 21, 2021
Comment on lines
+762
to
+763
| # happen when a module references another module (e.g. the criterion | ||
| # references the module), thus yielding that module's parameters again. |
There was a problem hiding this comment.
This could be silly question: What is an example of a criterion with learnable parameters?
There was a problem hiding this comment.
Not silly. One use case that has been discussed at some point is for GAN training with the generator being the module and the discriminator the criterion (though you might need different criteria in this case). Another case (which revealed this bug for me) is when using GPyTorch.
But this issue is not only relevant when the criterion references the module, it could also be one module referencing a second module.
thomasjpfan
approved these changes
Jun 22, 2021
BenjaminBossan
deleted the
bugfix/duplicate-params-when-modules-reference-each-other
branch
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.
When a net's module references another of that net's modules, the former
will yield the latter's parameters. Therefore, when all parameters are
collected, the latter's parameters appear twice.
This bugfix consists of remembering all yielded parameters in a set and
not yielding those that were already encountered.
This bug was introduced very recently (#751) and should occur very
rarely, since modules don't typically reference each other.