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

[Numpy] ffi invocation: expand_dims, tril, diff, broadcast_to #17738

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

Alicia1529
Copy link
Contributor

Description

as title

@Alicia1529 Alicia1529 requested a review from szha as a code owner March 2, 2020 09:14
@Alicia1529 Alicia1529 changed the title ffi invocation: expand_dims, tril, diff, broadcast_to [Numpy] ffi invocation: expand_dims, tril, diff, broadcast_to Mar 2, 2020
@haojin2 haojin2 added the Numpy label Mar 2, 2020
@haojin2 haojin2 self-assigned this Mar 2, 2020
src/api/operator/numpy/np_diff_op.cc Outdated Show resolved Hide resolved
src/api/operator/numpy/matrix_op.cc Outdated Show resolved Hide resolved
src/api/operator/numpy/matrix_op.cc Outdated Show resolved Hide resolved
src/api/operator/numpy/np_diff_op.cc Outdated Show resolved Hide resolved
src/api/operator/numpy/np_tril_op.cc Outdated Show resolved Hide resolved
src/api/operator/numpy/np_broadcast_reduce_op_value.cc Outdated Show resolved Hide resolved
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


int num_outputs = 0;
NDArray* inputs[] = {args[0].operator mxnet::NDArray*()};
auto ndoutputs = Invoke(op, &attrs, 1, inputs, &num_outputs, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

The 1 seems like a magic number. Better to use int num_inputs = 1 and call Invoke with num_inputs. Same for 3 other operators.

@haojin2 haojin2 merged commit 2cf8e0f into apache:master Mar 9, 2020
MoisesHer pushed a commit to MoisesHer/incubator-mxnet that referenced this pull request Apr 10, 2020
anirudh2290 pushed a commit to anirudh2290/mxnet that referenced this pull request May 29, 2020
sxjscience pushed a commit to sxjscience/mxnet that referenced this pull request Jul 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants