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

MXExecutorSimpleBindEx for backward compatability #7227

Closed
wants to merge 1 commit into from

Conversation

eric-haibin-lin
Copy link
Member

added MXExecutorSimpleBindEx since perl package is already using MXExecutorSimpleBind

@sergeykolychev
Copy link
Contributor

@eric-haibin-lin Thank you.
But if you prefer I can work on patch over weekend for the perl side to apply on your 'sparse' branch that will just jam nulls into these variables for now.
Let me know.

@reminisce
Copy link
Contributor

I agree that making the simple bind c-api consistent between front-end languages is more preferable.

@eric-haibin-lin
Copy link
Member Author

@sergeykolychev sparse tensor PR is a huge one. I think it will take a while before the sparse branch get fully reviewed and merged to master. For now I'm fine with an cpi compatible with perl and mark the old one deprecated. After perl moved from MXExecutorSimpleBind to MXExecutorSimpleBindEx, we can remove MXExecutorSimpleBind.

@eric-haibin-lin
Copy link
Member Author

eric-haibin-lin commented Jul 31, 2017

@sergeykolychev Oh I misunderstood your message. I thought you're going to make the change to master branch. If you can change the perl binding in sparse branch to use the modified simple_bind c_api with nulls, that will be great. Let me know when you have time to do this and i can close this PR

@sergeykolychev
Copy link
Contributor

@eric-haibin-lin I'll do it tomorrow

@eric-haibin-lin
Copy link
Member Author

awesome thank you very much!

@sergeykolychev
Copy link
Contributor

sergeykolychev commented Jul 31, 2017

@eric-haibin-lin
Can't do it easily, the sparse branch is very old and the perl state is as of May 14
Are you planning to merge master into sparse anytime soon ? If that is done I can fix perl in couple of hours after that.

@eric-haibin-lin
Copy link
Member Author

@sergeykolychev could you make a PR to this branch?
https://github.com/eric-haibin-lin/mxnet/tree/sparse

@sergeykolychev
Copy link
Contributor

@eric-haibin-lin Ok, this branch is much better, will work on it this evening.

@sergeykolychev
Copy link
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants