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

Required fixes #1

Open
chldkdtn opened this issue Aug 9, 2023 · 0 comments
Open

Required fixes #1

chldkdtn opened this issue Aug 9, 2023 · 0 comments

Comments

@chldkdtn
Copy link
Collaborator

chldkdtn commented Aug 9, 2023

Default values not properly declared

If the function takes a default value in its arguments, it needs to be specified with py::arg. For example of Vector constructor,

// Constructor
.def(py::init([](py::array_t<double> vec, bool distributed, bool copy_data = true) {
        py::buffer_info buf_info = vec.request();
        int dim = buf_info.shape[0];
        double* data = static_cast<double*>(buf_info.ptr);
        return new Vector(data, dim, distributed, copy_data);
    }), py::arg("vec"), py::arg("distributed"), py::arg("copy_data") = true) // default value needs to be defined here.

Note the last line.

Vector::getData not returning an array object

In c++, getData function returns the memory address of the data array. In python, this should return the reference of the data array. Currently, Vector::getData does not do any special care, returning a float value as a result.

.def("getData", &Vector::getData)

Vector::get_data naming

Compared to the function above, this function can cause a confusion. Based on its action, it needs to be renamed as copy_data or something.

(Enhancement) Vector and Matrix as a buffer protocol

Vector and Matrix should support buffer view, to handler large-size arrays without copying them in memory. Useful references are:

Based on pybind11 numpy instruction, an example of Matrix definition should be:

py::class_<Matrix>(m, "Matrix", py::buffer_protocol()) 
    .def_buffer([](Matrix &self) -> py::buffer_info {
        return py::buffer_info(
            self.getData(),                         /* Pointer to buffer */
            sizeof(float),                          /* Size of one scalar */
            py::format_descriptor<float>::format(), /* Python struct-style format descriptor */
            2,                                      /* Number of dimensions */
            { self.numRows(), self.numColumns() },  /* Buffer dimensions */
            { sizeof(float) * self.numColumns(),
              sizeof(float) }                       /* Strides (in bytes) for each index */
        );
    })

Similar definition is possible for Vector as well.

(Enhancement) Supporting slice view of Vector and Matrix

Vector and Matrix must support slice view of its data.

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

No branches or pull requests

1 participant