-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Hey @waytrue17 , Thanks for submitting the PR
CI supported jobs: [clang, miscellaneous, unix-gpu, edge, centos-cpu, unix-cpu, centos-gpu, windows-cpu, website, windows-gpu, sanity] Note: |
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.
Thanks for the quick implementation! left a few minor comments
python/mxnet/onnx/mx2onnx/_op_translations/_op_translations_opset12.py
Outdated
Show resolved
Hide resolved
@@ -1858,3 +1858,19 @@ def rand_check(out): | |||
def rand_check_nd(out): | |||
return rand_check(out.asnumpy()) | |||
op_export_test('sample_multinomial', M, [x], tmp_path, mx_map=rand_check_nd, onnx_map=rand_check) | |||
|
|||
|
|||
@pytest.mark.parametrize("dtype", ['float32', 'int32', 'int64']) |
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.
Is the customer use case covered by the unit tests?
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.
Customer has not shared their use case yet
python/mxnet/onnx/mx2onnx/_op_translations/_op_translations_opset13.py
Outdated
Show resolved
Hide resolved
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.
LGTM, thanks!
Description
ONNX support and unittest for _split_v2
Checklist
Essentials
Changes
Comments