convert numpy input params to dtype float and order 'F' automatically

using numpy.astype(...). No copy if the params are already in the correct dtype and storage order.

For a function
    f(Matrix A, Matrix B),
simply wrapping it to pyx as
   f(A.astype(float, order='F', copy=False), B.astype(float, order='F', copy=False))
won't work.
It produces a strange side-effect that the content of A is overwritten by B and the two inputs are the same (data address) inside the function!

This is because Cython decreases the ref count for the temporary variable resulted from A.astype(...) before generates the wrap for B.astype(...).
Hence, the A.astype temp var is probably reused for B.astype, and they were pointing to the same data address.

For that reason, we have to go a longer route and wrap it as:
  A = A.astype(float, order='F', copy=False)
  B = B.astype(float, order='F', copy=False)
  f(A, B)

For future ref., here is a sample of the wrongly generated code that wraps the JacobianFactor constructor:
Jacobian(Key i1, Matrix A1, Key i2, Matrix A2, Vector b, noiseModel::Diagonal model)

Wrongly wrapped pyx code:
self.shared_CJacobianFactor_ = shared_ptr[CJacobianFactor](new CJacobianFactor(i1, <MatrixXd>(Map[MatrixXd](A1.astype(float, order='F',copy=False)), i2, <MatrixXd>(Map[MatrixXd](A2.astype(float, order='F', copy=False)), <VectorXd>(Map[VectorXd](b.astype(float, order='F', copy=False))), model.shared_CnoiseModel_Diagonal_))

The problematic Cython generated CPP code with a comment on the problematic line:
/////////////////////////////////////////
// WRONG VERSION
/////////////////////////////////////////
  __pyx_t_12 = __Pyx_PyObject_GetAttrStr(((PyObject *)__pyx_v_A1), __pyx_n_s_astype); if (unlikely(!__pyx_t_12)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_12);
  __pyx_t_4 = PyTuple_New(1); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_4);
  __Pyx_INCREF(((PyObject *)(&PyFloat_Type)));
  __Pyx_GIVEREF(((PyObject *)(&PyFloat_Type)));
  PyTuple_SET_ITEM(__pyx_t_4, 0, ((PyObject *)(&PyFloat_Type)));
  __pyx_t_5 = PyDict_New(); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_5);
  if (PyDict_SetItem(__pyx_t_5, __pyx_n_s_order, __pyx_n_s_F) < 0) __PYX_ERR(0, 2107, __pyx_L1_error)
  if (PyDict_SetItem(__pyx_t_5, __pyx_n_s_copy, Py_False) < 0) __PYX_ERR(0, 2107, __pyx_L1_error)
  __pyx_t_13 = __Pyx_PyObject_Call(__pyx_t_12, __pyx_t_4, __pyx_t_5); if (unlikely(!__pyx_t_13)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_13);
  __Pyx_DECREF(__pyx_t_12); __pyx_t_12 = 0;
  __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
  __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
  if (!(likely(((__pyx_t_13) == Py_None) || likely(__Pyx_TypeTest(__pyx_t_13, __pyx_ptype_5numpy_ndarray))))) __PYX_ERR(0, 2107, __pyx_L1_error)
  try {
    __pyx_t_14 = eigency::Map<Eigen::MatrixXd> (((PyArrayObject *)__pyx_t_13));
  } catch(...) {
    __Pyx_CppExn2PyErr();
    __PYX_ERR(0, 2107, __pyx_L1_error)
  }
///////////////////////////////////////////////
  __Pyx_DECREF(__pyx_t_13); __pyx_t_13 = 0;  	//<------- Problematic line!!! Killing this will result in the correct result!
///////////////////////////////////////////////
  __pyx_t_13 = __Pyx_PyObject_GetAttrStr(((PyObject *)__pyx_v_A2), __pyx_n_s_astype); if (unlikely(!__pyx_t_13)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_13);
  __pyx_t_5 = PyTuple_New(1); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_5);
  __Pyx_INCREF(((PyObject *)(&PyFloat_Type)));
  __Pyx_GIVEREF(((PyObject *)(&PyFloat_Type)));
  PyTuple_SET_ITEM(__pyx_t_5, 0, ((PyObject *)(&PyFloat_Type)));
  __pyx_t_4 = PyDict_New(); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_4);
  if (PyDict_SetItem(__pyx_t_4, __pyx_n_s_order, __pyx_n_s_F) < 0) __PYX_ERR(0, 2107, __pyx_L1_error)
  if (PyDict_SetItem(__pyx_t_4, __pyx_n_s_copy, Py_False) < 0) __PYX_ERR(0, 2107, __pyx_L1_error)
  __pyx_t_12 = __Pyx_PyObject_Call(__pyx_t_13, __pyx_t_5, __pyx_t_4); if (unlikely(!__pyx_t_12)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_12);
  __Pyx_DECREF(__pyx_t_13); __pyx_t_13 = 0;
  __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
  __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
  if (!(likely(((__pyx_t_12) == Py_None) || likely(__Pyx_TypeTest(__pyx_t_12, __pyx_ptype_5numpy_ndarray))))) __PYX_ERR(0, 2107, __pyx_L1_error)
  try {
    __pyx_t_15 = eigency::Map<Eigen::MatrixXd> (((PyArrayObject *)__pyx_t_12));
  } catch(...) {
    __Pyx_CppExn2PyErr();
    __PYX_ERR(0, 2107, __pyx_L1_error)
  }
  __Pyx_DECREF(__pyx_t_12); __pyx_t_12 = 0;
  __pyx_t_12 = __Pyx_PyObject_GetAttrStr(((PyObject *)__pyx_v_b), __pyx_n_s_astype); if (unlikely(!__pyx_t_12)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_12);
  __pyx_t_4 = PyTuple_New(1); if (unlikely(!__pyx_t_4)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_4);
  __Pyx_INCREF(((PyObject *)(&PyFloat_Type)));
  __Pyx_GIVEREF(((PyObject *)(&PyFloat_Type)));
  PyTuple_SET_ITEM(__pyx_t_4, 0, ((PyObject *)(&PyFloat_Type)));
  __pyx_t_5 = PyDict_New(); if (unlikely(!__pyx_t_5)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_5);
  if (PyDict_SetItem(__pyx_t_5, __pyx_n_s_order, __pyx_n_s_F) < 0) __PYX_ERR(0, 2107, __pyx_L1_error)
  if (PyDict_SetItem(__pyx_t_5, __pyx_n_s_copy, Py_False) < 0) __PYX_ERR(0, 2107, __pyx_L1_error)
  __pyx_t_13 = __Pyx_PyObject_Call(__pyx_t_12, __pyx_t_4, __pyx_t_5); if (unlikely(!__pyx_t_13)) __PYX_ERR(0, 2107, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_13);
  __Pyx_DECREF(__pyx_t_12); __pyx_t_12 = 0;
  __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0;
  __Pyx_DECREF(__pyx_t_5); __pyx_t_5 = 0;
  if (!(likely(((__pyx_t_13) == Py_None) || likely(__Pyx_TypeTest(__pyx_t_13, __pyx_ptype_5numpy_ndarray))))) __PYX_ERR(0, 2107, __pyx_L1_error)
  try {
    __pyx_t_16 = eigency::Map<Eigen::VectorXd> (((PyArrayObject *)__pyx_t_13));
  } catch(...) {
    __Pyx_CppExn2PyErr();
    __PYX_ERR(0, 2107, __pyx_L1_error)
  }
  __Pyx_DECREF(__pyx_t_13); __pyx_t_13 = 0;
  try {
    __pyx_t_17 = new gtsam::JacobianFactor(__pyx_v_i1, ((Eigen::MatrixXd)__pyx_t_14), __pyx_v_i2, ((Eigen::MatrixXd)__pyx_t_15), ((Eigen::VectorXd)__pyx_t_16), __pyx_v_model->shared_CnoiseModel_Diagonal_);
  } catch(...) {
    __Pyx_CppExn2PyErr();
    __PYX_ERR(0, 2107, __pyx_L1_error)
  }
  __pyx_v_self->shared_CJacobianFactor_ = boost::shared_ptr<gtsam::JacobianFactor> (__pyx_t_17);
release/4.3a0
Duy-Nguyen Ta 2017-03-15 13:47:11 -04:00
parent 0e278f81c6
commit 6bf7ea23cf
6 changed files with 25 additions and 1 deletions

View File

@ -130,6 +130,13 @@ void Argument::emit_cython_pyx(FileWriter& file) const {
file.oss << type.pyxArgumentType() << " " << name;
}
/* ************************************************************************* */
std::string Argument::pyx_convertEigenTypeAndStorageOrder() const {
if (!type.isEigen())
return "";
return name + " = " + name + ".astype(float, order=\'F\', copy=False)";
}
/* ************************************************************************* */
std::string Argument::pyx_asParam() const {
string cythonType = type.pxdClassName();
@ -142,7 +149,7 @@ std::string Argument::pyx_asParam() const {
} else {
cythonVar = name;
}
return cythonVar;
return cythonVar;
}
/* ************************************************************************* */
@ -242,6 +249,14 @@ void ArgumentList::emit_cython_pyx(FileWriter& file) const {
}
}
/* ************************************************************************* */
std::string ArgumentList::pyx_convertEigenTypeAndStorageOrder(const std::string& indent) const {
string ret;
for (size_t j = 0; j < size(); ++j)
ret += indent + at(j).pyx_convertEigenTypeAndStorageOrder() + "\n";
return ret;
}
/* ************************************************************************* */
std::string ArgumentList::pyx_asParams() const {
string ret;

View File

@ -73,6 +73,7 @@ struct Argument {
const std::vector<std::string>& templateArgs) const;
void emit_cython_pyx(FileWriter& file) const;
std::string pyx_asParam() const;
std::string pyx_convertEigenTypeAndStorageOrder() const;
friend std::ostream& operator<<(std::ostream& os, const Argument& arg) {
os << (arg.is_const ? "const " : "") << arg.type << (arg.is_ptr ? "*" : "")
@ -131,6 +132,7 @@ struct ArgumentList: public std::vector<Argument> {
std::string pyx_asParams() const;
std::string pyx_paramsList() const;
std::string pyx_castParamsToPythonType() const;
std::string pyx_convertEigenTypeAndStorageOrder(const std::string& indent) const;
/**
* emit checking arguments to MATLAB proxy

View File

@ -148,6 +148,7 @@ void Constructor::emit_cython_pyx(FileWriter& pyxFile, const Class& cls) const {
pyxFile.oss << " def " + cls.pyxClassName() + "_" + to_string(i) +
"(self, *args, **kwargs):\n";
pyxFile.oss << pyx_resolveOverloadParams(args, true);
pyxFile.oss << argumentList(i).pyx_convertEigenTypeAndStorageOrder(" ");
pyxFile.oss << " self." << cls.shared_pxd_obj_in_pyx() << " = "
<< cls.shared_pxd_class_in_pyx() << "(new " << cls.pxd_class_in_pyx()

View File

@ -164,6 +164,7 @@ void GlobalFunction::emit_cython_pyx_no_overload(FileWriter& file) const {
file.oss << "):\n";
/// Call cython corresponding function and return
file.oss << argumentList(0).pyx_convertEigenTypeAndStorageOrder(" ");
string ret = pyx_functionCall("", pxdName(), 0);
if (!returnVals_[0].isVoid()) {
file.oss << " cdef " << returnVals_[0].pyx_returnType()
@ -200,6 +201,7 @@ void GlobalFunction::emit_cython_pyx(FileWriter& file) const {
file.oss << pyx_resolveOverloadParams(args, false, 1); // lazy: always return None even if it's a void function
/// Call cython corresponding function
file.oss << argumentList(i).pyx_convertEigenTypeAndStorageOrder(" ");
string ret = pyx_functionCall("", pxdName(), i);
if (!returnVals_[i].isVoid()) {
file.oss << " cdef " << returnVals_[i].pyx_returnType()

View File

@ -115,6 +115,7 @@ void Method::emit_cython_pyx_no_overload(FileWriter& file,
file.oss << "):\n";
/// Call cython corresponding function and return
file.oss << argumentList(0).pyx_convertEigenTypeAndStorageOrder(" ");
string caller = "self." + cls.shared_pxd_obj_in_pyx() + ".get()";
string ret = pyx_functionCall(caller, funcName, 0);
if (!returnVals_[0].isVoid()) {
@ -158,6 +159,7 @@ void Method::emit_cython_pyx(FileWriter& file, const Class& cls) const {
file.oss << pyx_resolveOverloadParams(args, false); // lazy: always return None even if it's a void function
/// Call cython corresponding function
file.oss << argumentList(i).pyx_convertEigenTypeAndStorageOrder(" ");
string caller = "self." + cls.shared_pxd_obj_in_pyx() + ".get()";
string ret = pyx_functionCall(caller, funcName, i);

View File

@ -79,6 +79,7 @@ void StaticMethod::emit_cython_pyx_no_overload(FileWriter& file,
file.oss << "):\n";
/// Call cython corresponding function and return
file.oss << argumentList(0).pyx_convertEigenTypeAndStorageOrder(" ");
string ret = pyx_functionCall(cls.pxd_class_in_pyx(), name_, 0);
file.oss << " ";
if (!returnVals_[0].isVoid()) {
@ -117,6 +118,7 @@ void StaticMethod::emit_cython_pyx(FileWriter& file, const Class& cls) const {
file.oss << pyx_resolveOverloadParams(args, false); // lazy: always return None even if it's a void function
/// Call cython corresponding function and return
file.oss << argumentList(i).pyx_convertEigenTypeAndStorageOrder(" ");
string ret = pyx_functionCall(cls.pxd_class_in_pyx(), pxdFuncName, i);
if (!returnVals_[i].isVoid()) {
file.oss << " cdef " << returnVals_[i].pyx_returnType()