xtensor-stack / xtensor

C++ tensors with broadcasting and lazy computing
BSD 3-Clause "New" or "Revised" License
3.31k stars 392 forks source link

Creating an xtensor_container from memory #1937

Open papadop opened 4 years ago

papadop commented 4 years ago

I facing with the following problem: I have class like this (simplified):

class XMatrix: public xt::xtensor<double,2> {

        typedef xt::xtensor<double,2>  base;
        typedef XXX                             Matrix;

        template <typename T>
        T& f(T&& ref) { std::cerr << "BASE: " << &(ref(0,0)) << std::endl; return ref; }

    public:
        using base::base;

        XMatrix(SPMatrix& M): base(f(xt::adapt(M.data(),{ M.rows(), M.columns() })))
        { std::cerr << "HERE " << M.data() << " " << &((*this)(0,0)) << std::endl; }
};

The problem is that the adaptator is well constructed to point to the data of the passed matrix, but the data are copied when initializing the base constructor (aka xtensor<double,2>). There seems no simple way to construct directly the xtensor from a memory buffer (and not owning it). Am I missing something ?

The execution shows sthg like this: BASE: 0x61b000027680 HERE 0x61b000027680 0x61b000028480

Is there any workaround I can use to bypass this copy ??

emmenlau commented 4 years ago

Hello @papadop , this may or may not be related to your problem but there is a potential pitfall in your code: its not recommended to inherit from the xtensor-containers. This is due to the https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern.

emmenlau commented 4 years ago

Other than that, do the xtensor adapters from https://xtensor.readthedocs.io/en/latest/external-structures.html help?

papadop commented 4 years ago

But in practice, a class from which you cannot derive is less useful. I agree it is not recommended, but if you want to use a subset of all the facilities, it is a convenient way of doing things. I have used blitz (another CRTP based matrix class) deriving from it, and it works quite well for the use I have for it.

In my case, I have a simple matrix container because most of the time I only need to access elements... In some cases, I want to do some limited linear algebra or access to some columns or submatrices and then I need a more capable tool.

In this case, what hurts me is the copy constructor that copies data when an xtensor is initialized from a xt:adapter. I probably need to add an xtensor constructor that allows me to use an already allocated buffer. But this maybe not feasible with the xtensor philosophy...

JohanMabille commented 4 years ago

But in practice, a class from which you cannot derive is less useful.

It's not. That's the point of value semantics, you provide a class that is copiable and assignable, with a well defined set of methods. Public inheritance from a class with value semantics leads to a lot of potential issues (slicing in assignment oprator, incomplete copy, etc).

I agree it is not recommended, but if you want to use a subset of all the facilities, it is a convenient way of doing things.

That is a terrible practice. A better way is to use private inheritance and using declarations. This way you avoid all the issues described before (slicing, incomplete copy, etc). Classes meant to be public base classes implement entity semantics. Or value semantics but in protected section, so they can never be instantiated directly (CRTP base classes for instance).

In the cas of xtensor, as pointed out by @emmenlau , the base class is a CRTP class, so its argument should be the most inheriting class in the hierarchy. Otherwise, you may run into the slicing and incomplete issues mentioned before. If your class does not add any data member, inheriting from xtensor is fine as long as it remains a private base class.

In some cases, I want to do some limited linear algebra or access to some columns or submatrices and then I need a more capable tool.

xtensor provides views, and xtensor-blas provides a lot of linear algebra tools. Which tool do you need that is missing from them?

In this case, what hurts me is the copy constructor that copies data when an xtensor is initialized from a xt:adapter. I probably need to add an xtensor constructor that allows me to use an already allocated buffer.

These (xtensor and xtensor_adaptor) are different classes with different memory model and different memory management. The adapter may take ownership of the adapted buffer, or not, while an xtensor object is always responsible of its memory. An adapter holds a reference on adapted containers, meaning it is not default constructible while xtensor is always default constructible.

emmenlau commented 4 years ago

@papadop I was also a happy user of blitz++ and I agree it was nice in many aspects. I do not mean to compare the two and also not to change your mind. But for us, switching to xtensor was quite a win. Its very actively maintained, and we found it to be a lot faster in certain scenarios (for example iterators have shown to be many orders of magnitudes faster for us).

For the specific issue of inheritance, I was at first unhappy about this too. But it turned out that after a careful inspection of our use case, a container class for the tensor is actually equally useful and provides better encapsulation of the data (and by this I mean its less likely to be mis-used because it exposes only well-defined operators).

Maybe if you review your use case xtensor can be useful for you too? In any case, feel free to discuss your needs, also in the gitter chat.

SylvainCorlay commented 4 years ago

Following discussions with @JohanMabille, I think that we should make these xtensor classes final so that this kind of inheritance is explicitly prohibited.