Skip to content
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

[T.F 2.0 API Docs] Adding details for acosh math function #29016

Merged
merged 6 commits into from Jun 6, 2019
Merged

[T.F 2.0 API Docs] Adding details for acosh math function #29016

merged 6 commits into from Jun 6, 2019

Conversation

SSaishruthi
Copy link
Contributor

@SSaishruthi SSaishruthi commented May 24, 2019 •
edited

Referring to issue #25802
Added:

  • Description
  • Input range
  • Example

In order to reflect in doc, should I change in any file?

Thanks.

@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label May 24, 2019
@SSaishruthi SSaishruthi changed the title Adding doc for 2.0 acosh [T.F 2.0 API] Adding doc for 2.0 acosh May 24, 2019
@SSaishruthi SSaishruthi changed the title [T.F 2.0 API] Adding doc for 2.0 acosh [T.F 2.0 API] Adding doc for acosh May 24, 2019
@SSaishruthi
Copy link
Contributor Author

@dynamicwebpaige This address the issue #25802.
Working on other components

@SSaishruthi SSaishruthi changed the title [T.F 2.0 API] Adding doc for acosh [T.F 2.0 API Docs] Adding details for acosh math function May 25, 2019
@gbaned gbaned self-assigned this May 25, 2019
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation May 25, 2019
@gbaned gbaned requested a review from martinwicke May 25, 2019 08:44
martinwicke
martinwicke requested changes May 31, 2019
View reviewed changes
tensorflow/core/api_def/base_api/api_def_Acosh.pbtxt Outdated
Given an input tensor, the function computes inverse hyperbolic cosine of every element.
Input range is [1, infinity). It returns `nan` if the input lies outside the range.
```python
tensor `x` is [-2, -0.5, 1, 1.2, 200, 10000]
Copy link
Member

Choose a reason for hiding this comment

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

Please write examples as code, in this case:

x = [-2, -0.5, 1, 1.2, 200, 10000]

tensorflow/core/api_def/base_api/api_def_Acosh.pbtxt Show resolved Hide resolved
tensorflow/core/api_def/base_api/api_def_Acosh.pbtxt Outdated
@@ -1,4 +1,11 @@
op {
graph_op_name: "Acosh"
summary: "Computes inverse hyperbolic cosine of x element-wise."
description: <<END
Given an input tensor, the function computes inverse hyperbolic cosine of every element.
Input range is [1, infinity). It returns `nan` if the input lies outside the range.
Copy link
Member

Choose a reason for hiding this comment

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

I'd say inf instead of infinity

Please put backticks around [1, inf) so it renders as code.

tensorflow/core/api_def/base_api/api_def_Acosh.pbtxt Outdated
Given an input tensor, the function computes inverse hyperbolic cosine of every element.
Input range is [1, infinity). It returns `nan` if the input lies outside the range.
```python
tensor `x` is [-2, -0.5, 1, 1.2, 200, 10000]
Copy link
Member

Choose a reason for hiding this comment

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

might be good to add inf to x, so readers know what happens.

PR Queue automation moved this from Assigned Reviewer to Reviewer Requested Changes May 31, 2019
@SSaishruthi
Copy link
Contributor Author

Thanks, @martinwicke for the review
I have added inf as one of the inputs. Is it ok to give as float("inf") or I need to use math.inf?

martinwicke
martinwicke requested changes Jun 1, 2019
View reviewed changes
tensorflow/core/api_def/base_api/api_def_Acosh.pbtxt Outdated
@@ -3,9 +3,10 @@ op {
summary: "Computes inverse hyperbolic cosine of x element-wise."
description: <<END
Given an input tensor, the function computes inverse hyperbolic cosine of every element.
Input range is [1, infinity). It returns `nan` if the input lies outside the range.
Input range is `[1, inf)`. It returns `nan` if the input lies outside the range.
Copy link
Member

Choose a reason for hiding this comment

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

If the output below is correct, then the input range is [1, inf], since inf -> inf, not inf -> nan.

tensorflow/core/api_def/base_api/api_def_Acosh.pbtxt Show resolved Hide resolved
@martinwicke
Copy link
Member

float(inf) is fine, I left some more comments. Please also apply the equivalent to your other PRs if they appear relevant.

@SSaishruthi
Copy link
Contributor Author

@martinwicke Completed changes. Please let me know if this looks good.

PR Queue automation moved this from Reviewer Requested Changes to Approved by Reviewer Jun 1, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jun 1, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jun 1, 2019
@tensorflow-copybara tensorflow-copybara merged commit 2ddc71d into tensorflow:master Jun 6, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Jun 6, 2019
tensorflow-copybara pushed a commit that referenced this pull request Jun 6, 2019
@tensorflower-gardener
Merge pull request #29016 from SSaishruthi:cosh_doc
1a3b22f 
PiperOrigin-RevId: 251881337
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinwicke martinwicke martinwicke approved these changes

Assignees

@gbaned gbaned

Labels
cla: yes ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
PR Queue
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@SSaishruthi @martinwicke @googlebot @kokoro-team @tensorflow-copybara @gbaned