-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Hey @JiangZhaoh , Thanks for submitting the PR
CI supported jobs: [website, centos-gpu, centos-cpu, sanity, windows-cpu, unix-cpu, unix-gpu, windows-gpu, clang, edge, miscellaneous] Note: |
Is it using the new FFI? |
37380d4
to
0c376d2
Compare
Related to #17823 |
It seems that this operation currently only supports floating type. In addition, it would be great if the data types of indices can be extended to import mxnet as mx
a = mx.np.zeros((2,6))
val = mx.np.arange(6)
x = [1,0,0,0,0,0]
y = [0,2,3,4,0,1]
# indices = [list, list]
mx.npx.index_add(a, val, [x, y])
# indices = (list, list)
mx.npx.index_add(a, val, (x, y))
# indices = [np.ndarray, np.ndarray]
x = mx.np.array(x).astype('int32')
y = mx.np.array(y).astype('int32')
mx.npx.index_add(a, val, [x, y])
>>> MXNetError: MXNetError: Invalid Parameter format for ind expect tuple of <Shape(tuple)>
but value='[array([1, 0, 0, 0, 0, 0], dtype=int32), array([0, 2, 3, 4, 0, 1], dtype=int32)]', in
operator _npx_index_add(name="", ind="[array([1, 0, 0, 0, 0, 0], dtype=int32), array([0, 2, 3, 4, 0, 1], dtype=int32)]")
|
3e06269
to
6a54c64
Compare
For |
LGTM overall. Needs to fix the CudaMalloc + cudaFree. |
@mxnet-bot run ci [unix-gpu, windows-cpu, windows-gpu] |
Jenkins CI successfully triggered : [windows-cpu, windows-gpu, unix-gpu] |
eb80c3e
to
40a14d9
Compare
@mxnet-bot run ci [unix-gpu, unix-cpu, centos-gpu] |
Jenkins CI successfully triggered : [unix-cpu, unix-gpu, centos-gpu] |
ab33786
to
1a01058
Compare
830e101
to
a6b1350
Compare
ret.emplace_back(a_grad); | ||
ret.emplace_back(idx_grad); | ||
ret.emplace_back(val_grad); | ||
return ret; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yzhliu I've suggested @JiangZhaoh to write the gradient op by reusing the forward op. This has multiple benefits:
- We can do more optimization in the graph pass
- It's easy to support higher-order gradient.
I think we may add the document about such usage since not many people are aware of that.
@mxnet-bot run ci [centos-gpu] [unix-cpu] [unix-gpu] [windows-cpu] [windows-gpu] |
Jenkins CI successfully triggered : [centos-gpu] |
@mxnet-bot run ci [unix-cpu, unix-gpu, windows-cpu, windows-gpu] |
Jenkins CI successfully triggered : [unix-cpu, windows-gpu, windows-cpu, unix-gpu] |
Thanks @JiangZhaoh , this is merged. |
* part cpu * index_add forward & test * fix wrong doc * fix index_add_sanity_error * index_update_test * remove index_update & implement index_add backward * fix sanity error * reduce code length * depart into two file * test CI compiler * test CI * test CI * reduce mshadow & allow more dtype * fix sanity error * fix conflict * reduce fwd macro code * reduce bwd macro code * fix compile error * tensor ind * remove cudaMalloc/cudaFree * fix windows compile error * fix compile error * use value instead of references * remove pragma * fix naive engine error * try to pass CI * fix sanity error * depart gradient into three node * resolve comment & initialize mshadow::Shape * fix werror Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Xingjian Shi <[email protected]>
* part cpu * index_add forward & test * fix wrong doc * fix index_add_sanity_error * index_update_test * remove index_update & implement index_add backward * fix sanity error * reduce code length * depart into two file * test CI compiler * test CI * test CI * reduce mshadow & allow more dtype * fix sanity error * fix conflict * reduce fwd macro code * reduce bwd macro code * fix compile error * tensor ind * remove cudaMalloc/cudaFree * fix windows compile error * fix compile error * use value instead of references * remove pragma * fix naive engine error * try to pass CI * fix sanity error * depart gradient into three node * resolve comment & initialize mshadow::Shape * fix werror Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Xingjian Shi <[email protected]>
* part cpu * index_add forward & test * fix wrong doc * fix index_add_sanity_error * index_update_test * remove index_update & implement index_add backward * fix sanity error * reduce code length * depart into two file * test CI compiler * test CI * test CI * reduce mshadow & allow more dtype * fix sanity error * fix conflict * reduce fwd macro code * reduce bwd macro code * fix compile error * tensor ind * remove cudaMalloc/cudaFree * fix windows compile error * fix compile error * use value instead of references * remove pragma * fix naive engine error * try to pass CI * fix sanity error * depart gradient into three node * resolve comment & initialize mshadow::Shape * fix werror Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Ubuntu <[email protected]> Co-authored-by: Xingjian Shi <[email protected]>
Description
Forward for index_add.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments