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.

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

[TF 2.0 API Docs] tf.math.acos #25802

Open
dynamicwebpaige opened this issue Feb 17, 2019 · 8 comments
Open

[TF 2.0 API Docs] tf.math.acos #25802

dynamicwebpaige opened this issue Feb 17, 2019 · 8 comments

Comments

@dynamicwebpaige
Copy link
Member

@dynamicwebpaige dynamicwebpaige commented Feb 17, 2019

System information

Describe the documentation issue

Documentation for tf.math.acos is created from a generated file, python/ops/gen_math_ops.py. It would be excellent to have a link to the files that are used to generate gen_math_ops.py - so a user could make modifications quickly, without having to search through tensorflow/tensorflow.

tf.math.acos could use a clear description, usage examples, and example visuals. A great template to model this on could be numpy.arccos.

Users also experience obfuscated errors from unexpected arguments (ex: strings, Booleans, and even just ints). Some examples are shared below. The file used to generate this error is located here. All tf.math.* operations and the operations they influence (e.g., tf.linspace) would experience these same obfuscated XLA errors.

InvalidArgumentError: Invalid cast from floating point type to S32 in ConstantR0WithType.
	 [[{{node Acos}}]] [Op:Acos]
InternalError: Could not find valid device for node.
Node: {{node Acos}}
All kernels registered for op Acos :
  device='XLA_CPU'; T in [DT_FLOAT, DT_DOUBLE, DT_INT32, DT_COMPLEX64, DT_INT64, DT_COMPLEX128, DT_HALF]
  device='XLA_CPU_JIT'; T in [DT_FLOAT, DT_DOUBLE, DT_INT32, DT_COMPLEX64, DT_INT64, DT_COMPLEX128, DT_HALF]
  device='CPU'; T in [DT_DOUBLE]
  device='CPU'; T in [DT_FLOAT]
 [Op:Acos]

We welcome contributions by users. Will you be able to update submit a PR (use the doc style guide) to fix the doc Issue?
I shall certainly try! :)

@Sudeepam97
Copy link
Contributor

@Sudeepam97 Sudeepam97 commented Feb 19, 2019

Hi @dynamicwebpaige. I searched through the tensorflow repo, and it looks like one needs to edit the .pbtxt files inside tensorflow/tensorflow/core/api_def/base_api to modify the documentation. Can you confirm this?

@dynamicwebpaige
Copy link
Member Author

@dynamicwebpaige dynamicwebpaige commented Feb 21, 2019

@Sudeepam97 The gen_math_ops.py file is generated from tensorflow.bzl. The C++ code located here defines documentation for each math op -- an example of how to modify the description via .Doc can be found here. 🙂

cc: @MarkDaoust @lamberta to keep me honest, though! Do we have a recommended process for updating those docs?

@dynamicwebpaige
Copy link
Member Author

@dynamicwebpaige dynamicwebpaige commented Feb 24, 2019

@Sudeepam97 Just confirmed that you are correct! Apologies for the confusion - the .pbtxtfiles in /base_api still create the documentation for several of the TF 2.0 math endpoints.

Let me take a look at your PR; we might need to tweak the syntax a little bit (format is a special flavor of markdown). Check out this endpoint for a usage example.

@Sudeepam97
Copy link
Contributor

@Sudeepam97 Sudeepam97 commented Feb 24, 2019

@dynamicwebpaige, that's great! Please take your time to review the PR, I'll be happy to work on any changes that are necessary. 😄
I seem to understand how to modify the API docs now. I can work on other similar issues as well.

@SSaishruthi
Copy link
Contributor

@SSaishruthi SSaishruthi commented May 28, 2019

@dynamicwebpaige
Created 5 PRs covering some of the math functions. Working on other functions.

HarshSulakhe added a commit to HarshSulakhe/tensorflow that referenced this issue Aug 25, 2019
HarshSulakhe added a commit to HarshSulakhe/tensorflow that referenced this issue Aug 25, 2019
@parasgoyal12
Copy link

@parasgoyal12 parasgoyal12 commented Nov 23, 2019

I would love contributing to this. Please tell me how to proceed as I am newcomer.

@rohanreddych
Copy link
Contributor

@rohanreddych rohanreddych commented Mar 17, 2020

@SSaishruthi are there any ops left to do

@mihaimaruseac
Copy link
Collaborator

@mihaimaruseac mihaimaruseac commented Mar 17, 2020

Just a note for people working on this: if you add python doctests, please make sure to do so in Python files, not in base_api/api_def files as those are the APIs for other languages too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
TensorFlow 2.0
  
Awaiting triage
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
@mihaimaruseac @dynamicwebpaige @SSaishruthi @Sudeepam97 @parasgoyal12 @rohanreddych @jvishnuvardhan and others
You can’t perform that action at this time.