Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Fix exception handling api doc #13519

Merged
merged 2 commits into from Dec 8, 2018

Conversation

anirudh2290
Copy link
Member

Description

There were some issues with MXNet users because of not being aware of waitall not rethrowing exceptions. It was documented as a limitation in exception handling doc here: https://mxnet.incubator.apache.org/architecture/exception_handling.html?highlight=exception#limitation but not in the API doc.
Adding it to the API doc.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • API doc change

@@ -157,6 +157,11 @@ def waitall():
"""Wait for all async operations to finish in MXNet.

This function is used for benchmarking only.
Copy link
Member

Choose a reason for hiding this comment

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

Not many people will understand what Rethrowing exception means. I suggest the below:

. warning:: This API is for internal purpose for benchmarking) , it does not do any error handling and can cause silent failures. Use it only if you are confident that your code error free, suggested to call wait_to_read on all outputs after waitall.

@nswamy nswamy added Unclear Error/Doc Doc pr-awaiting-response PR is reviewed and waiting for contributor to respond labels Dec 7, 2018
Copy link
Contributor

@aaronmarkham aaronmarkham left a comment

Choose a reason for hiding this comment

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

Adjusting Naveen's comment.
Please also test this with make html USE_OPENMP=1 SPHINXOPTS=-W to make sure the special warning syntax is formatted correctly. For example, my suggestion may merge without a line break after that warning block and that could be a problem...

python/mxnet/ndarray/ndarray.py Outdated Show resolved Hide resolved
Co-Authored-By: anirudh2290 <anirudh2290@apache.org>
@anirudh2290
Copy link
Member Author

Thanks @aaronmarkham @nswamy . Updated the API doc.

@nswamy nswamy merged commit 7d2b804 into apache:master Dec 8, 2018
zhaoyao73 pushed a commit to zhaoyao73/incubator-mxnet that referenced this pull request Dec 13, 2018
* Fix exception handling api doc

* Update waitall api doc

Co-Authored-By: anirudh2290 <anirudh2290@apache.org>
zhaoyao73 added a commit to zhaoyao73/incubator-mxnet that referenced this pull request Dec 13, 2018
* upstream/master: (54 commits)
  Add notes about debug with libstdc++ symbols (apache#13533)
  add cpp example inception to nightly test (apache#13534)
  Fix exception handling api doc (apache#13519)
  fix link for gluon model zoo (apache#13583)
  ONNX import/export: Size (apache#13112)
  Update MXNetTutorialTemplate.ipynb (apache#13568)
  fix the situation where idx didn't align with rec (apache#13550)
  Fix use-before-assignment in convert_dot (apache#13511)
  License update  (apache#13565)
  Update version to v1.5.0 including clojure package (apache#13566)
  Fix flaky test test_random:test_randint_generator (apache#13498)
  Add workspace cleaning after job finished (apache#13490)
  Adding test for softmaxoutput (apache#13116)
  apache#13441 [Clojure] Add Spec Validations for the Random namespace (apache#13523)
  Revert "Bumped minor version from 1.4.0 to 1.5.0 on master, updated License file" (apache#13558)
  Chi_square_check for discrete distribution fix (apache#13543)
  Updated docs for randint operator (apache#13541)
  Simplifications and some fun stuff for the MNIST Gluon tutorial (apache#13094)
  Fix apache#13521 (apache#13537)
  Add a retry to qemu_provision (apache#13551)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Doc pr-awaiting-response PR is reviewed and waiting for contributor to respond Unclear Error/Doc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants