KEMBAR78
[doc][hackathon] To add Adam Optimizer to the documentation by iramazanli · Pull Request #63251 · pytorch/pytorch · GitHub
Skip to content

[doc][hackathon] To add Adam Optimizer to the documentation #63251

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[doc][hackathon] To add Adam Optimizer to the documentation #63251

wants to merge 1 commit into from

Conversation

@iramazanli
Copy link
Contributor

@iramazanli iramazanli commented Aug 13, 2021 •
edited by pytorch-probot bot
Loading

It has been discussed before that adding description of Optimization algorithms to PyTorch Core documentation may result in a nice Optimization research tutorial. In the following tracking issue we mentioned about all the necessary algorithms and links to the originally published paper #63236.

In this PR we are adding description of Adam Algorithm to the documentation. For more details, we refer to the paper https://arxiv.org/abs/1412.6980

Screen Shot 2021-08-27 at 6 37 54 PM

cc @vincentqb @iramazanli

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 13, 2021 •
edited
Loading

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 6626eb5 (more details on the Dr. CI page):

  • 3/3 failures introduced in this PR

🕵️ 3 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

See CircleCI build pytorch_linux_xenial_py3_6_gcc5_4_build (1/3)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result. 
CONFLICT (add/add): Merge conflict in .github/scripts/generate_ci_workflows.py
Auto-merging .github/scripts/generate_ci_workflows.py
CONFLICT (add/add): Merge conflict in .github/scripts/ensure_actions_will_cancel.py
Auto-merging .github/scripts/ensure_actions_will_cancel.py
CONFLICT (add/add): Merge conflict in .github/generated-ciflow-ruleset.json
Auto-merging .github/generated-ciflow-ruleset.json
CONFLICT (add/add): Merge conflict in .circleci/config.yml
Auto-merging .circleci/config.yml
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_data.py
Auto-merging .circleci/cimodel/data/pytorch_build_data.py
Automatic merge failed; fix conflicts and then commit the result.


Exited with code exit status 1

See CircleCI build pytorch_xla_linux_bionic_py3_6_clang9_build (2/3)

Step: "(Optional) Merge target branch" (full log | diagnosis details | 🔁 rerun)

Automatic merge failed; fix conflicts and then commit the result. 
CONFLICT (add/add): Merge conflict in .github/scripts/generate_ci_workflows.py
Auto-merging .github/scripts/generate_ci_workflows.py
CONFLICT (add/add): Merge conflict in .github/scripts/ensure_actions_will_cancel.py
Auto-merging .github/scripts/ensure_actions_will_cancel.py
CONFLICT (add/add): Merge conflict in .github/generated-ciflow-ruleset.json
Auto-merging .github/generated-ciflow-ruleset.json
CONFLICT (add/add): Merge conflict in .circleci/config.yml
Auto-merging .circleci/config.yml
CONFLICT (add/add): Merge conflict in .circleci/cimodel/data/pytorch_build_data.py
Auto-merging .circleci/cimodel/data/pytorch_build_data.py
Automatic merge failed; fix conflicts and then commit the result.


Exited with code exit status 1

See GitHub Actions build win-vs2019-cpu-py3 / test (default, 1, 2, windows.4xlarge) (3/3)

Step: "Run test scripts" (full log | diagnosis details | 🔁 rerun)

2021-09-07T17:49:24.5837444Z ERROR [0.000s]: test_poisson_sample (__main__.TestDistributions) 
2021-09-07T17:49:24.5831495Z   File "distributions/test_distributions.py", line 812, in _check_sampler_discrete
2021-09-07T17:49:24.5832140Z     chisq, p = scipy.stats.chisquare(counts[msk], pmf[msk] * num_samples)
2021-09-07T17:49:24.5832850Z   File "c:\jenkins\miniconda3\lib\site-packages\scipy\stats\stats.py", line 6852, in chisquare
2021-09-07T17:49:24.5833518Z     return power_divergence(f_obs, f_exp=f_exp, ddof=ddof, axis=axis,
2021-09-07T17:49:24.5834214Z   File "c:\jenkins\miniconda3\lib\site-packages\scipy\stats\stats.py", line 6694, in power_divergence
2021-09-07T17:49:24.5834808Z     raise ValueError(msg)
2021-09-07T17:49:24.5835604Z ValueError: For each axis slice, the sum of the observed frequencies must agree with the sum of the expected frequencies to a relative tolerance of 1e-08, but the percent differences are:
2021-09-07T17:49:24.5836317Z 0.008265582255680495
2021-09-07T17:49:24.5836500Z 
2021-09-07T17:49:24.5837012Z ======================================================================
2021-09-07T17:49:24.5837444Z ERROR [0.000s]: test_poisson_sample (__main__.TestDistributions)
2021-09-07T17:49:24.5837993Z ----------------------------------------------------------------------
2021-09-07T17:49:24.5838426Z Traceback (most recent call last):
2021-09-07T17:49:24.5839024Z   File "distributions/test_distributions.py", line 1352, in test_poisson_sample
2021-09-07T17:49:24.5839618Z     self._check_sampler_discrete(Poisson(rate),
2021-09-07T17:49:24.5840226Z   File "distributions/test_distributions.py", line 812, in _check_sampler_discrete
2021-09-07T17:49:24.5840917Z     chisq, p = scipy.stats.chisquare(counts[msk], pmf[msk] * num_samples)
2021-09-07T17:49:24.5841656Z   File "c:\jenkins\miniconda3\lib\site-packages\scipy\stats\stats.py", line 6852, in chisquare
2021-09-07T17:49:24.5842300Z     return power_divergence(f_obs, f_exp=f_exp, ddof=ddof, axis=axis,
2021-09-07T17:49:24.5843020Z   File "c:\jenkins\miniconda3\lib\site-packages\scipy\stats\stats.py", line 6694, in power_divergence
2021-09-07T17:49:24.5843675Z     raise ValueError(msg)
This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@iramazanli iramazanli force-pushed the adam_algorithm_doc branch 2 times, most recently from 5970814 to 64543f0 Compare August 13, 2021 19:38
@iramazanli iramazanli force-pushed the adam_algorithm_doc branch 8 times, most recently from 42f20bd to fa7c43c Compare August 15, 2021 17:52
@iramazanli iramazanli changed the title [doc][hackathon]To add Adam Optimizer to the documentation [doc][hackathon] To add Adam Optimizer to the documentation Aug 15, 2021
@iramazanli iramazanli requested a review from albanD August 15, 2021 18:13
@iramazanli iramazanli force-pushed the adam_algorithm_doc branch 2 times, most recently from af59471 to fc67b05 Compare August 24, 2021 18:05
@codecov
Copy link

codecov bot commented Aug 24, 2021 •
edited
Loading

Codecov Report

Merging #63251 (fc67b05) into master (f0d2742) will decrease coverage by 0.08%.
The diff coverage is n/a.

❗ Current head fc67b05 differs from pull request most recent head 44c88c5. Consider uploading reports for the commit 44c88c5 to get more accurate results

@@            Coverage Diff             @@
##           master   #63251      +/-   ##
==========================================
- Coverage   67.09%   67.00%   -0.09%     
==========================================
  Files         692      691       -1     
  Lines       90579    90558      -21     
==========================================
- Hits        60774    60680      -94     
- Misses      29805    29878      +73

@iramazanli iramazanli force-pushed the adam_algorithm_doc branch 3 times, most recently from 44c88c5 to 09d0911 Compare August 27, 2021 22:49
albanD
albanD approved these changes Sep 7, 2021
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

LGTM

torch/optim/adam.py Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not true anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This paper contains AdamW rather than Adam, that's why i deleted this :) thanks for pointing it out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

@facebook-github-bot
Copy link
Contributor

@iramazanli has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@iramazanli has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@iramazanli merged this pull request in 43248d9.

@Damowerko Damowerko mentioned this pull request Mar 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albanD albanD albanD approved these changes

Assignees

No one assigned

Labels

cla signed docs-hackathon hackathon Merged module: optimizer Related to torch.optim

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iramazanli @facebook-github-bot @albanD