Skip to content

[Update transforms.py]: use build-in atanh in TanhTransform#40160

Closed
zuoxingdong wants to merge 1 commit intopytorch:masterfrom
zuoxingdong:patch-7
Closed

[Update transforms.py]: use build-in atanh in TanhTransform#40160
zuoxingdong wants to merge 1 commit intopytorch:masterfrom
zuoxingdong:patch-7

Conversation

@zuoxingdong
Copy link
Copy Markdown
Contributor

Since torch.atanh is recently implemented in #38388, we should simply use it for TanhTransform.

@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Jun 17, 2020

💊 CI failures summary and remediations

As of commit 081e131 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed


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 on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 1 time.

@zuoxingdong
Copy link
Copy Markdown
Contributor Author

Had a look into the console log file, it seems that the error comes from test_openmp failed, not sure if that is relevant to this PR.

@zou3519
Copy link
Copy Markdown
Contributor

zou3519 commented Jun 23, 2020

@fritzo, @neerajprad -- github is suggesting you two as reviewers, could you take a look please?


@staticmethod
def atanh(x):
return 0.5 * (x.log1p() - (-x).log1p())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am concerned that the code looks like this for numeric stability reasons, but there is no comment to that effect here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's right. Specially for values close to 0, this will be more accurate than the 0.5*log((1+x)/(1-x)) formulation.

@zou3519 zou3519 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 23, 2020
@neerajprad
Copy link
Copy Markdown
Contributor

LGTM. The failure is unrelated.

@pytorchbot merge this please.

@pytorchbot pytorchbot added the merge-this-please Was marked for merge with @pytorchbot merge this please label Jun 23, 2020
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ezyang merged this pull request in f13653d.

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
…40160)

Summary:
Since `torch.atanh` is recently implemented in pytorch#38388, we should simply use it for `TanhTransform`.
Pull Request resolved: pytorch#40160

Differential Revision: D22208039

Pulled By: ezyang

fbshipit-source-id: 34dfbc91eb9383461e16d3452e3ebe295f39df26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-this-please Was marked for merge with @pytorchbot merge this please Merged open source triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants