Add dynamic matrix #21
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| #pragma once | ||
|
|
||
| #ifndef DKM_MATRIX_HPP | ||
| #define DKM_MATRIX_HPP | ||
|
|
||
| #include <vector> | ||
| #include <cassert> | ||
| #include <algorithm> | ||
|
|
||
| namespace dkm | ||
| { | ||
| // This class is only an interface! Not designed to be used outside library internals. | ||
|
|
||
| template<typename T> | ||
| class as_matrix | ||
| { | ||
| const T *data = nullptr; | ||
|
|
||
| // column major indexer | ||
| auto cm_indexer(size_t i, size_t j) const -> const T& | ||
| { | ||
| assert(i >= 0 && j >= 0 && i < n_rows && j < n_cols); | ||
| return data[j * n_rows + i]; | ||
| } | ||
|
|
||
| // row major indexer | ||
| auto rm_indexer(size_t i, size_t j) const -> const T& | ||
| { | ||
| assert(i >= 0 && j >= 0 && i < n_rows && j < n_cols); | ||
| return data[i * n_cols + j]; | ||
| } | ||
|
|
||
| const T& (as_matrix<T>::*indexer) (size_t, size_t) const = nullptr; | ||
|
|
||
| public: | ||
| const size_t n_rows, n_cols; | ||
|
|
||
| as_matrix(const T *data, size_t n_rows, size_t n_cols, bool col_major = true) | ||
|
Owner
There was a problem hiding this comment. It would be great to see a constructor for conversion from the existing
Author
There was a problem hiding this comment. We can instead overload the main function and mark the old version as
Owner
There was a problem hiding this comment. This approach is also acceptable, as long as |
||
| : data(data), n_rows(n_rows), n_cols(n_cols), | ||
| indexer((col_major)? &as_matrix<T>::cm_indexer: &as_matrix<T>::rm_indexer) | ||
| {} | ||
|
|
||
| auto row(size_t i) const -> std::vector<T>; | ||
| auto operator()(size_t i, size_t j) const -> const T& | ||
|
Owner
There was a problem hiding this comment. I would prefer this was just a named function like |
||
| { | ||
| return ((this)->*(this->indexer))(i, j); | ||
| } | ||
| }; | ||
|
|
||
|
|
||
| template<typename T> | ||
| auto as_matrix<T>::row(size_t i) const -> std::vector<T> | ||
| { | ||
| auto res = std::vector<T>(n_cols); | ||
| for(size_t j = 0; j < n_cols; j++) | ||
| { | ||
| res[j] = (*this)(i, j); | ||
|
Owner
There was a problem hiding this comment. One of the reasons this library is so performant is that it avoids allocations and copies wherever possible. This is a full copy of each and every row in the input data, multiple times. It will have a significant effect on performance. The way to do this performantly and safely would be to return a
Author
There was a problem hiding this comment. I had been implementing a new version that takes this into account. The idea is whenever the matrix is column major we first copy (because we cannot change the incoming data) and transpose it. The copy would work like this: auto [owner, data] = copy(matrix);From here we can safely return the span. See that the matrix data structure never owns the data, so we need to return an owner (arguably I plan to implement this when my project reaches the optimization stage. Here is an implementation I was already working on https://pastebin.com/DYQs2EBb
Owner
There was a problem hiding this comment. Why can transposition not just produce a new There's no reason to have any solution here that includes any form of copying.
Author
There was a problem hiding this comment. The transposition is performed only once right after the function is called. I didn't measure the performance yet but I think that the overhead is insignificant. The idea with transposition is that with column major data the next element of a given row is at a distance of n_rows. It is more performant and easier to work with if we keep the elements at a distance of 1.
Owner
There was a problem hiding this comment. My argument was mainly that in this case the representation of the data doesn't need to change at all, we just need need a different view over the data, which seems to be the whole point of
Author
There was a problem hiding this comment. I understand your point, my only concern is that in the case of row major data to get a row we can easily return auto row_vec::operator()(size_t i)
{
return data[i * n_rows + this->row_number]
}From my experience the transposing the data + changing from column major to row major is simpler and faster, specially when the input data is large enough. But I am not completely sure if this is true in fact. What is your opinion on this? |
||
| } | ||
| return res; | ||
| } | ||
|
|
||
| } | ||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be fine to just name this
matrix.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name comes from the fact that we are dealing with the data "as a matrix". But
as_matrixis in fact not even a matrix (in common sense), it is only an interface between the algorithm and the data. I think the nameas_matrixexpresses better the idea.