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

[Numpy] FFI: random.choice, take and clip #17854

Merged
merged 27 commits into from
Apr 13, 2020
Merged

[Numpy] FFI: random.choice, take and clip #17854

merged 27 commits into from
Apr 13, 2020

Conversation

AntiZpvoh
Copy link
Contributor

Description

Add FFI support of

  • random.choice

  • take

  • clip

  Old FFI (ctypes) (us) New FFI (cython) (us)
random.choice 70.73 23.31
take 61.6 29.95
clip 31.95 11.44

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • 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 https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

AntiZpvoh and others added 21 commits March 3, 2020 16:14
@AntiZpvoh
change the header file of np.random.choice
eb1de03
@AntiZpvoh
add np_choice_op.cc file
0e3f461
@AntiZpvoh
Merge branch 'master' of https://github.com/apache/incubator-mxnet in…
5c83b9b 
…to NumPyFFI
@AntiZpvoh
add including header file
2201b07
@AntiZpvoh
implement the basic function of random.choice
f2d84e8
@AntiZpvoh
try to use take op in backend
ab688ab
@AntiZpvoh
try to use take op in backend
e5aef6f
@AntiZpvoh
add take invoking function
69df45c
@AntiZpvoh
fix some syntax problems
ce34578
@AntiZpvoh
fix some problems
76392a2
@AntiZpvoh
complete numpy.random.choice ffi
815d2c4
@AntiZpvoh
merge with origin
cbcbd9e
@AntiZpvoh
first commit of ffi indexing_op.cc
b242984
add random.choice ffi benchmark
c497ce0
@AntiZpvoh
Merge branch 'NumPyFFI' of https://github.com/AntiZpvoh/incubator-mxnet…
ac8b83f 
… into npFFI
@AntiZpvoh
complete take ffi
9ac3558
@AntiZpvoh
change the implementation of random.choice
b20a98f
add take op benchmark
9e1281e
@AntiZpvoh
complete clip op ffi and fix a problem
760f234
@AntiZpvoh
Merge branch 'master' of https://github.com/apache/incubator-mxnet in…
dba7be9 
…to npFFI
add clip op benchmark
f431437
@AntiZpvoh AntiZpvoh requested a review from szha as a code owner March 17, 2020 06:48
@haojin2 haojin2 added the Numpy label Mar 17, 2020
@haojin2
Copy link
Contributor

haojin2 commented Mar 17, 2020

please also fix the sanity issues.

hzfan
hzfan reviewed Mar 19, 2020
View reviewed changes
python/mxnet/ndarray/numpy/_op.py Outdated Show resolved Hide resolved
src/api/operator/tensor/indexing_op.cc Outdated Show resolved Hide resolved
src/api/operator/tensor/matrix_op.cc Outdated Show resolved Hide resolved
src/api/operator/numpy/random/np_choice_op.cc Outdated Show resolved Hide resolved
hzfan
hzfan approved these changes Mar 23, 2020
View reviewed changes
Copy link
Contributor

@hzfan hzfan left a comment

Choose a reason for hiding this comment

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

LGTM

@mxnet-bot
Copy link

None of the jobs entered are supported.
Jobs entered by user: [ci/jenkins/mxnet-validation/centos-cpu, ci/jenkins/mxnet-validation/unix-gpu]
CI supported Jobs: [centos-cpu, windows-gpu, unix-gpu, edge, windows-cpu, sanity, clang, centos-gpu, website, miscellaneous, unix-cpu]

@AntiZpvoh
Copy link
Contributor Author

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

@mxnet-bot
Copy link

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

@AntiZpvoh
Copy link
Contributor Author

@mxnet-bot run ci [unix-gpu]

@mxnet-bot
Copy link

Jenkins CI successfully triggered : [unix-gpu]

@haojin2 haojin2 merged commit e3d7866 into apache:master Apr 13, 2020
sxjscience pushed a commit to sxjscience/mxnet that referenced this pull request Jul 1, 2020
@AntiZpvoh @sxjscience
[Numpy] FFI: random.choice, take and clip (apache#17854)
1944c70 
* change the header file of np.random.choice

* add np_choice_op.cc file

* add including header file

* implement the basic function of random.choice

* try to use take op in backend

* try to use take op in backend

* add take invoking function

* fix some syntax problems

* fix some problems

* complete numpy.random.choice ffi

* first commit of ffi indexing_op.cc

* add random.choice ffi benchmark

* complete take ffi

* change the implementation of random.choice

* add take op benchmark

* complete clip op ffi and fix a problem

* add clip op benchmark

* fix some sanity problems

* add space before ( and fix reimport

* fix a typo

* remove dead code and remove new operator

Co-authored-by: Ubuntu <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@haojin2 haojin2 haojin2 left review comments

@hzfan hzfan hzfan approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
Numpy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AntiZpvoh @haojin2 @mxnet-bot @hzfan