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

Use DEFAULT macro in C APIs #14767

Merged
merged 2 commits into from
Apr 24, 2019
Merged

Use DEFAULT macro in C APIs #14767

merged 2 commits into from
Apr 24, 2019

Conversation

yuxihu
Copy link
Member

@yuxihu yuxihu commented Apr 22, 2019

This PR changes the way of specifying the default values in C APIs by using the DEFAULT macro. Otherwise, it will fail compilation with C code. We also remove the duplicate declaration of MXInvokeCachedOp API.

@yuxihu
Copy link
Member Author

yuxihu commented Apr 22, 2019

@frankfliu Please help review.

Copy link
Contributor

@frankfliu frankfliu left a comment

Choose a reason for hiding this comment

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

LGTM

@yuxihu
Copy link
Member Author

yuxihu commented Apr 23, 2019

@mxnet-label-bot add [pr-awaiting-review]

@marcoabreu marcoabreu added the pr-awaiting-review PR is waiting for code review label Apr 23, 2019
@yuxihu
Copy link
Member Author

yuxihu commented Apr 24, 2019

@wkcn Could you please help review and merge? Thanks.

Copy link
Contributor

@larroy larroy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

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

LGTM

@wkcn
Copy link
Member

wkcn commented Apr 24, 2019

Thank you for the fix!

Why MXInvokeCachedOp is duplicated?
https://github.com/apache/incubator-mxnet/blob/225f71f744ac5e7bd29868b6d3ba0e4fe2527c43/python/mxnet/cython/ndarray.pyx and https://github.com/apache/incubator-mxnet/blob/3f3ba92ae1468d08de088d2291ca14e2d5dc5515/perl-package/AI-MXNetCAPI/mxnet.i call this function.

Edit:
I see. There are some problems on the github searcher.

@wkcn wkcn merged commit 587d480 into apache:master Apr 24, 2019
@wkcn
Copy link
Member

wkcn commented Apr 24, 2019

Merged. Thank you!

@yuxihu yuxihu deleted the c_api_fix branch April 24, 2019 23:40
yuxihu added a commit to yuxihu/incubator-mxnet that referenced this pull request Apr 24, 2019
* fix C API default values

* remove duplicate MXInvokeCachedOp
szha pushed a commit that referenced this pull request Apr 25, 2019
* fix C API default values

* remove duplicate MXInvokeCachedOp
haohuanw pushed a commit to haohuanw/incubator-mxnet that referenced this pull request Jun 23, 2019
* fix C API default values

* remove duplicate MXInvokeCachedOp
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr-awaiting-review PR is waiting for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants