Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Frontend][MXNet] ones zeros ones_like zeros_like ops support #1814

Merged
merged 4 commits into from
Oct 21, 2018

Conversation

imorinaga
Copy link
Contributor

This PR supports 6 symbols in MXNet frontend.

@srkreddy1238 @nishi-t Could you please do the review. Thanks.

@nishi-t
Copy link
Contributor

nishi-t commented Oct 4, 2018

@imorinaga Could you rebase against the most recent master?

@imorinaga
Copy link
Contributor Author

@nishi-t Thank you for comments! rebase done.

@nishi-t
Copy link
Contributor

nishi-t commented Oct 5, 2018

please add tests for these operators

@yzhliu
Copy link
Member

yzhliu commented Oct 15, 2018

@imorinaga Any updates?

@yzhliu yzhliu added the status: need update need update based on feedbacks label Oct 15, 2018
@imorinaga
Copy link
Contributor Author

argmax and argmin support is deleted.
Output dtype of these are fp32 in MXNet.
However, these are int32 in NNVM, Tensorflow, coreml and onnx.

@nishi-t
Copy link
Contributor

nishi-t commented Oct 16, 2018

Thank you for adding tests. If the point you have pointed out is a testing problem for argmin and argmax, #1908 may be able to fix it.

@imorinaga
Copy link
Contributor Author

imorinaga commented Oct 18, 2018

Thank you for comment!
What I mean is not a testing interface problem.
To support argmax Symbol, I think out_dtype interface is needed on nnvm.symbol.argmax.

@nishi-t
Copy link
Contributor

nishi-t commented Oct 18, 2018

@imorinaga OK. I don't have sure knowledge of the return type and couldn't confirm it from the document. But, If argmin and argmax return the indices as float32 in mxnet, we would need to discuss how to support it.
BTW, In this PR, do you intend to support only operators excluding argmin and argmax?

@imorinaga imorinaga closed this Oct 19, 2018
@imorinaga imorinaga reopened this Oct 19, 2018
@imorinaga
Copy link
Contributor Author

@nishi-t
Yes, I want to support only ones, ones_like, zeros and zeros_like in this PR. Should I change the PR title?
BTW, I accidentally closed this PR so I've reopened. Sorry for confusing.

Copy link
Contributor

@nishi-t nishi-t left a comment

Choose a reason for hiding this comment

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

@imorinaga Ok, Thanks. LGTM

@nishi-t
Copy link
Contributor

nishi-t commented Oct 19, 2018

@imorinaga yes, I'd suggest changing the title.

@imorinaga imorinaga changed the title [Frontend][MXNet] ones zeros ones_like zeros_like argmax argmin ops support [Frontend][MXNet] ones zeros ones_like zeros_like ops support Oct 19, 2018
@tqchen tqchen merged commit 150f7a8 into apache:master Oct 21, 2018
@tqchen tqchen added status: accepted and removed status: need update need update based on feedbacks labels Oct 21, 2018
@imorinaga imorinaga deleted the mxnet-symbol branch October 21, 2018 22:22
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants