Skip to content

[docs] Add images to some activation functions#65415

Closed
rodrigoberriel wants to merge 1 commit intopytorch:masterfrom
rodrigoberriel:add-images-actv-func
Closed

[docs] Add images to some activation functions#65415
rodrigoberriel wants to merge 1 commit intopytorch:masterfrom
rodrigoberriel:add-images-actv-func

Conversation

@rodrigoberriel
Copy link
Contributor

@rodrigoberriel rodrigoberriel commented Sep 21, 2021 •
edited
Loading

Fixes #65368. See discussion in the issue.

cc @mruberry @ssnl @jbschlosser @soulitzer

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 21, 2021 •
edited
Loading

🔗 Helpful links

💊 CI failures summary and remediations

As of commit 462ee84 (more details on the Dr. CI page):

💚 💚 Looks good so far! There are no failures yet. 💚 💚

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.

albanD
albanD reviewed Sep 21, 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.

I think you forgot to add the images to the commit?

@rodrigoberriel
Copy link
Contributor Author

rodrigoberriel commented Sep 21, 2021

I think you forgot to add the images to the commit?

AFAIK, they are generated on-the-fly:

pytorch/docs/Makefile

Lines 16 to 17 in 00b732e

figures:
@$(PYCMD) source/scripts/build_activation_images.py

@albanD
Copy link
Collaborator

albanD commented Sep 21, 2021

Ho interesting, I didn't knew we had such a script :p
The rendered doc looks good: https://docs-preview.pytorch.org/65415/generated/torch.nn.SiLU.html?highlight=silu#torch.nn.SiLU

albanD
albanD approved these changes Sep 21, 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 thanks!

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 21, 2021

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

@soulitzer soulitzer self-requested a review September 21, 2021 21:12
@codecov
Copy link

codecov bot commented Sep 21, 2021

Codecov Report

Merging #65415 (00b732e) into master (7f6580a) will increase coverage by 0.14%.
The diff coverage is 50.00%.

❗ Current head 00b732e differs from pull request most recent head 462ee84. Consider uploading reports for the commit 462ee84 to get more accurate results

@@            Coverage Diff             @@
##           master   #65415      +/-   ##
==========================================
+ Coverage   66.24%   66.38%   +0.14%     
==========================================
  Files         738      738              
  Lines       94182    94182              
==========================================
+ Hits        62387    62522     +135     
+ Misses      31795    31660     -135

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 22, 2021

@albanD merged this pull request in 11ca641.

@rodrigoberriel rodrigoberriel deleted the add-images-actv-func branch September 22, 2021 18:21
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

@soulitzer soulitzer soulitzer approved these changes

@jbschlosser jbschlosser jbschlosser approved these changes

Assignees

No one assigned

Labels

cla signed Merged open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[docs] Some activation functions are missing images

6 participants

@rodrigoberriel @facebook-github-bot @albanD @soulitzer @jbschlosser @pytorchbot