Coder Social home page Coder Social logo

Comments (6)

RUrlus avatar RUrlus commented on July 23, 2024 1

Thanks for the MRE, I could reproduce. A bit surprised this slipped through considering the number of testing there is.
It's fixed in the unstable branch now.

If you're curious, for some reason I decided that we needed to copy if the array was smaller than Armadillo's pre-allocation limits whilst that only needs to happen when the is also stolen.
The garbage values in the first two examples happened due some very strange stride issue for c-style arrays.

from carma.

conradsnicta avatar conradsnicta commented on July 23, 2024 1

@RUrlus For safety reasons, perhaps the default operation should be copying data when going between Python and Armadillo.

Copying data is a relatively "cheap" operation, in contrast to operations such as matrix multiplication and SVD. I suspect that in practice the overhead of copying would be minimal. For folks that do want the extra speed, perhaps there should be an option to enable a "don't copy data" mode.

from carma.

RUrlus avatar RUrlus commented on July 23, 2024 1

@conradsnicta You're right its relatively cheap but it prevents borrowing on the conversion in which is a quite common pattern. Perhaps the default on the way out should become to copy but it's quite a change.

For integration into MLPack I need to make some changes regarding the copy behaviour anyway so I might include a mode as you suggested.

from carma.

RUrlus avatar RUrlus commented on July 23, 2024

Yes you can modify in-place, at least with the manual conversion you always could. There was a bug in automatic conversion where an assignment triggered a copy rather than a move assignment even though it was an r-value. This is fixed in bfe4324.

What I mean in the documentation regarding the borrow and copying out is that you're not supposed to return the borrowed array as then you have two Numpy arrays pointing to the same memory and one will destruct the memory before the other.

from carma.

kjohnsen avatar kjohnsen commented on July 23, 2024

Ok, now that I know this is supported I'll try to make an MRE

from carma.

kjohnsen avatar kjohnsen commented on July 23, 2024

Ok, got some weird stuff--apparently the same thing on both stable and unstable branches.

Here's the binding code:

#include <pybind11/numpy.h>
#include <pybind11/pybind11.h>
#include <armadillo>
#include <carma>

// MRE for modifying as side-effect
void modify_arr(py::array_t<double>& arr) {
  arma::Mat<double> mat = carma::arr_to_mat(arr); // <-- I believe I got all the borrow syntax right
  mat(0,0) = 42;
  // std::cout << mat;  // <-- confirmed that the first element was getting set to 42 as expected
}

PYBIND11_MODULE(example, m) {
  m.def("modify_arr", &modify_arr);
}

Here's what happened in a Python interpreter. Looks like identity matrices get mangled and others remain un-modified:

>>> import numpy as np, example
>>> x = np.eye(3)
>>> example.modify_arr(x)
>>> x
array([[1.00000000e+000, 1.35675106e-312, 8.08686649e-320],
       [1.77658241e-307, 5.29980882e-315, 3.11261357e-322],
       [3.47328271e-310, 2.07023782e-317, 0.00000000e+000]])
>>> y = np.eye(3)
>>> example.modify_arr(y)
>>> y
array([[1.00000000e+000, 1.35675106e-312, 8.08686649e-320],
       [1.77658241e-307, 5.29980882e-315, 3.11261357e-322],
       [3.47328271e-310, 2.07023782e-317, 0.00000000e+000]])
>>> y = np.ones((3,3))
>>> example.modify_arr(y)
>>> y
array([[1., 1., 1.],
       [1., 1., 1.],
       [1., 1., 1.]])
>>> y=np.array([[1,2],[3,4]])
>>> example.modify_arr(y)
>>> y
array([[1, 2],
       [3, 4]])

from carma.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.