-
Notifications
You must be signed in to change notification settings - Fork 614
Implementing Gradient Accumulation Optimizer #2527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
def GradientAccumulator( | ||
optimizer: types.Optimizer, | ||
accu_steps: int = 2, | ||
trainable_variables=None | ||
) -> types.Optimizer: | ||
if trainable_variables is None: | ||
trainable_variables = list() | ||
|
||
if isinstance(optimizer, str): | ||
optimizer = tf.keras.optimizers.get(optimizer) | ||
|
||
optimizer.gradient_transformers.append( | ||
AccumulationGradientTransformer( | ||
optimizer=optimizer, | ||
accu_steps=accu_steps, | ||
trainable_variables=trainable_variables | ||
) | ||
) | ||
|
||
return optimizer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fsx950223 This here is for now just a function. Hence I disabled the test_config()
for now.
# def test_config(): | ||
# sgd_opt = tf.keras.optimizers.SGD(lr=2.0, nesterov=True, momentum=0.3, decay=0.1) | ||
# accum_steps = 4 | ||
# opt = GradientAccumulator(sgd_opt, accum_steps=accum_steps) | ||
# config = opt.get_config() | ||
# | ||
# assert config["accum_steps"] == accum_steps | ||
# | ||
# new_opt = GradientAccumulator.from_config(config) | ||
# old_sgd_config = opt._optimizer.get_config() | ||
# new_sgd_config = new_opt._optimizer.get_config() | ||
# | ||
# for k1, k2 in zip(old_sgd_config, new_sgd_config): | ||
# assert old_sgd_config[k1] == new_sgd_config[k2] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #2527 (comment)
@fsx950223 With this implementation I do not get any errors on my side. The (ASR) model I seek to train appears to converge. |
Do you know the reason? |
The reason why it works? No idea 😆 |
I can only assume that it's because internally I never have any branching or something like that 🤷♂️ I'll admit that I am not really an expert for the internal workings of Tensorflow (yet). But if you want I can make this PR on your forked repository on https://github.com/fsx950223/addons/tree/ga ? |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
for _ in range(accu_steps * 4): | ||
opt.apply_gradients(grads_and_vars) | ||
np.testing.assert_allclose( | ||
var0.read_value(), [[1.0, 2.0, 0.0], [0.2, 1.2, 0.0]], rtol=1e-5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove rtol=1e-5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why but the result has some rounding errors. I need to increase the tolerance here for the test to succeed. The gradients are computed correctly but the variables are updated with rounding errors.
tf.keras.optimizers.SGD(lr=1.0), accum_steps, trainable_variables=variables | ||
) | ||
|
||
for _ in range(accum_steps + 1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accum_steps*2+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You suggest to change this to:
for _ in range(accum_steps + 1): | |
for _ in range(accum_steps * 2 + 1): |
?
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
Close temporarily for deduplicate notes. |
Description
This PR is a fork/alternative of #2525 and seeks to implement gradient accumulation.
Type of change
Checklist: