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

[WIP] C-API <--> Python API for paddle.framework #2759

Closed

Conversation

reyoung
Copy link
Collaborator

@reyoung reyoung commented Jul 6, 2017

  • Add new C-API for refactored Paddle, in paddle/c
  • Expose that C-API to paddle.v2.framework.c.Variable and
    paddle.v2.framework.c.Scope
    • Use Cython to warp them and give a native Python API.
  • Add Unit tests.
    • See python/paddle/v2/framework/tests/test_scope.py for usage.

@reyoung reyoung changed the title C-API <--> Python API for paddle.framework [WIP] C-API <--> Python API for paddle.framework Jul 6, 2017
@reyoung reyoung force-pushed the feature/expose_c_api_for_framework branch from 1ed252c to 3765e46 Compare July 6, 2017 10:18
Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

Do we really need the C API?

I think that for the MVP, all we need is a Python API. And the Go API doesn't need to have as many features as the Python API #2750 (comment). Is this correct? Or, what are supposed to be exposed via the Go API?

SCOPE = 0,
};

#define PADDLE_HANDLE_HEADER ::paddle::c::ObjectType type_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个文件是干啥呢?

@@ -0,0 +1,32 @@
#include "scope.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we use Boost::Python, all the following files in this PR

  • scope.h
  • scope.cc
  • private.h
  • error.h
  • dllexports.h

would be replaced by the following few-line C++ code:

#include <boost/python.hpp>

BOOST_PYTHON_MODULE(pypaddle) {
    boost::python::class_<paddle::framework::Scope>("Scope")
        .def("create_var", &Scope::CreateVar)
        .def("create_scope", &Scope::CreateScope);
}

Please re-think about this necessity of create a C-API.

Copy link
Contributor

@gangliao gangliao Jul 7, 2017

Choose a reason for hiding this comment

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

Pybind11 is also an excellent candidate
https://github.com/pybind/pybind11
https://media.readthedocs.org/pdf/pybind11/master/pybind11.pdf

Caffe2 chose pybind11 to expose c++11 directly.

Copy link
Collaborator Author

@reyoung reyoung Jul 10, 2017

Choose a reason for hiding this comment

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

Whether uses C-API depends on is there any other languages need invoke Paddle C++ Core or not.

I am not sure only Python API is enough or not. At least there are several needs for us to give a C-API.

  • Android/IOS API. Give a Java/Objective-C wrapper for Paddle.
  • Go API. Needs for our Parameter Server.

Also, I think pybind11 is better than boost::Python because Paddle is in C++ 11 .

But if we have a C-API for Paddle, wrap that C-API to Python is extremely easy by Cython.

cdef extern from "math.h":
    double sin(double x)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This PR is just an exploring PR for how to expose Paddle C++ core by C-API & Cython. I will give another PR by using pybind11.

* Add new C-API for refactored Paddle, in `paddle/c`
* Expose that C-API to paddle.v2.framework.c.Variable and
  paddle.v2.framework.c.Scope
  * Use Cython to warp them and give a native Python API.
* Add Unit tests.
  * See `python/paddle/v2/framework/tests/test_scope.py` for usage.
@reyoung reyoung force-pushed the feature/expose_c_api_for_framework branch from 3765e46 to 4ab407f Compare July 10, 2017 06:11
@reyoung reyoung closed this Jul 13, 2017
@reyoung reyoung deleted the feature/expose_c_api_for_framework branch October 28, 2017 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants