Skip to content

Rephrase documentation of adabelief #2704

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 18, 2022
Merged

Conversation

denadai2
Copy link
Contributor

This PR fixes the documentation of total_steps, which enables the warmup only if the value is bigger than zero. However, the documentation says "positive value". Zero (the default value) is positive though

@google-cla
Copy link

google-cla bot commented May 18, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@bot-of-gabrieldemarmiesse

@juntang-zhuang

You are owner of some files modified in this pull request.
Would you kindly review the changes whenever you have the time to?
Thank you very much.

@@ -113,7 +113,7 @@ def __init__(
rectify: boolean. Whether to apply learning rate rectification as
from RAdam.
total_steps: An integer. Total number of training steps. Enable
warmup by setting a positive value.
warmup by setting a value greater than zero.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it was correct:
http://oeis.org/A000027
http://oeis.org/A001477

Copy link
Contributor Author

@denadai2 denadai2 May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. However, "greater than zero" I think requires less expertise to be understood. I thought zero was a positive number for example, and I have a PhD.

Copy link
Contributor

@bhack bhack May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be some confusion with signed zero but on integer I think it is hard to be confused. I will merge this as it is just an alternative.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@bhack bhack changed the title Fix documentation of adabelief Rephrase documentation of adabelief May 18, 2022
@bhack bhack merged commit e5bfef1 into tensorflow:master May 18, 2022
@juntang-zhuang
Copy link
Contributor

Looks great to me. The same issue is with rectified_adam document

Enable warmup by setting a positive value.
, you might consider modifying it as well.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants