You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2020/08/29 00:24:08 UTC

[GitHub] [incubator-mxnet] mk-61 opened a new pull request #19036: [WIP] AMP lists for numpy ops

mk-61 opened a new pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036


   ## Description ##
   1. Add all registered ops (mostly Numpy) to AMP lists
   2. Removed "skipped" attribute from AMP tests
   
   ## Comments ##
   - Do not merge, yet - untested.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698436152






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-699754061


   Jenkins CI successfully triggered : [windows-gpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-699754036


   @mxnet-bot run ci [windows-gpu, macosx-x86_64]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: [WIP] AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-694595694


   Jenkins CI successfully triggered : [unix-cpu, centos-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-699091958


   @mxnet-bot run ci [centos-cpu, unix-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-699092008


   Jenkins CI successfully triggered : [unix-cpu, centos-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] szha merged pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
szha merged pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698436187






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: [WIP] AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-694595665


   @mxnet-bot run ci [centos-cpu, unix-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698436187


   Jenkins CI successfully triggered : [unix-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: [WIP] AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-697540498


   Jenkins CI successfully triggered : [windows-cpu, unix-gpu, windows-gpu, sanity, clang, centos-gpu, website, centos-cpu, edge, unix-cpu, miscellaneous]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698687602


   @mxnet-bot run ci [centos-cpu, unix-cpu, unix-gpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: [WIP] AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-697540222


    @mxnet-bot run ci [all]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] ptrendx commented on a change in pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
ptrendx commented on a change in pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#discussion_r498465581



##########
File path: tests/python/gpu/test_contrib_amp.py
##########
@@ -66,45 +66,39 @@ def test_amp_coverage(amp_tests):
     assert ret == [], "Elements " + str(ret) + " exist in more than 1 AMP list."
 
     # Check the coverage
-    py_str = lambda x: x.decode('utf-8')
-
-    plist = ctypes.POINTER(ctypes.c_char_p)()
-    size = ctypes.c_uint()
-
-    mx.base._LIB.MXListAllOpNames(ctypes.byref(size),
-                                     ctypes.byref(plist))
-    op_names = []
-    for i in range(size.value):
-        s = py_str(plist[i])
-        if not s.startswith("_backward") \
-           and not s.startswith("_contrib_backward_"):
-            op_names.append(s)
-
-    ret1 = set(op_names) - set(t)
-
-    if ret1 != set():
-        warnings.warn("Operators " + str(ret1) + " do not exist in AMP lists (in "
-                       "python/mxnet/contrib/amp/lists/symbol_fp16.py) - please add them. "
-                       """Please follow these guidelines for choosing a proper list:
-                       - if your operator is not to be used in a computational graph
-                         (e.g. image manipulation operators, optimizers) or does not have
-                         inputs, put it in FP16_FP32_FUNCS list,
-                       - if your operator requires FP32 inputs or is not safe to use with lower
-                         precision, put it in FP32_FUNCS list,
-                       - if your operator supports both FP32 and lower precision, has
-                         multiple inputs and expects all inputs to be of the same
-                         type, put it in WIDEST_TYPE_CASTS list,
-                       - if your operator supports both FP32 and lower precision and has
-                         either a single input or supports inputs of different type,
-                         put it in FP16_FP32_FUNCS list,
-                       - if your operator is both safe to use in lower precision and
-                         it is highly beneficial to use it in lower precision, then
-                         put it in FP16_FUNCS (this is unlikely for new operators)
-                       - If you are not sure which list to choose, FP32_FUNCS is the
-                         safest option""")
+    covered = set(t)
+    ops = get_all_registered_operators_grouped()
+    required = set(k for k in ops
+                   if not k.startswith(("_backward", "_contrib_backward", "_npi_backward")) and
+                   not k.endswith("_backward"))
+
+    extra = covered - required
+    assert not extra, f"{len(extra)} operators are not needed in the AMP lists: {sorted(extra)}"
+
+    guidelines = """Please follow these guidelines for choosing a proper list:
+    - if your operator is not to be used in a computational graph
+      (e.g. image manipulation operators, optimizers) or does not have
+      inputs, put it in FP16_FP32_FUNCS list,
+    - if your operator requires FP32 inputs or is not safe to use with lower
+      precision, put it in FP32_FUNCS list,
+    - if your operator supports both FP32 and lower precision, has
+      multiple inputs and expects all inputs to be of the same
+      type, put it in WIDEST_TYPE_CASTS list,
+    - if your operator supports both FP32 and lower precision and has
+      either a single input or supports inputs of different type,
+      put it in FP16_FP32_FUNCS list,
+    - if your operator is both safe to use in lower precision and
+      it is highly beneficial to use it in lower precision, then
+      put it in FP16_FUNCS (this is unlikely for new operators)
+    - If you are not sure which list to choose, FP32_FUNCS is the
+                     safest option"""
+    diff = required - covered
+    assert not diff, f"{len(diff)} operators {sorted(diff)} do not exist in AMP lists (in " \
+        f"python/mxnet/contrib/amp/lists/symbol_fp16.py) - please add them. " \
+        f"\n{guidelines}"
 
 @with_seed()
-@pytest.mark.skip(reason='Error during waitall(). Tracked in #18099')
+@pytest.mark.skip(reason="WIP")

Review comment:
       ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: [WIP] AMP lists for numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-683201329


   Hey @mk-61 , Thanks for submitting the PR 
   All tests are already queued to run once. If tests fail, you can trigger one or more tests again with the following commands: 
   - To trigger all jobs: @mxnet-bot run ci [all] 
   - To trigger specific jobs: @mxnet-bot run ci [job1, job2] 
   *** 
   **CI supported jobs**: [sanity, centos-gpu, miscellaneous, windows-cpu, windows-gpu, website, unix-cpu, centos-cpu, edge, clang, unix-gpu]
   *** 
   _Note_: 
    Only following 3 categories can trigger CI :PR Author, MXNet Committer, Jenkins Admin. 
   All CI tests must pass before the PR can be merged. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mxnet-bot commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mxnet-bot commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698687636


   Jenkins CI successfully triggered : [unix-gpu, unix-cpu, centos-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] szha commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
szha commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698064932


   ```
   [2020-09-24T01:42:24.553Z] ************* Module mxnet.contrib.amp.loss_scaler
   
   [2020-09-24T01:42:24.553Z] python/mxnet/contrib/amp/loss_scaler.py:45:4: C0111: Missing method docstring (missing-docstring)
   
   [2020-09-24T01:42:24.553Z] python/mxnet/contrib/amp/loss_scaler.py:52:8: W0105: String statement has no effect (pointless-string-statement)
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-mxnet] mk-61 commented on pull request #19036: AMP support for Numpy ops

Posted by GitBox <gi...@apache.org>.
mk-61 commented on pull request #19036:
URL: https://github.com/apache/incubator-mxnet/pull/19036#issuecomment-698436152


   @mxnet-bot run ci [unix-cpu]


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org