Refactoring Tensor

changed milestone to %v0.1.0

added 1 deleted label

requested review from @pineapple and @olivierbichler

changed the description

added 1 commit

963e131d - [WIP][NF][BUILD] ongoing debugging memory corruptions

Compare with previous version

added 29 commits

963e131d...51af88a0 - 27 commits from branch main
0b58b387 - [MERGE][master] updating from main
fc8dff77 - [WIP][NF][BUILD] adding strict aliasing compiler option (the warning option...

Compare with previous version

One general comment: I think you are trying to change maybe too deeply the initial concept of Tensor and TensorImpl. TensorImpl was supposed to be a very thin object that would provide device data storage and access, according to the specified dimensions of Tensor. In this vision, Tensor would not just be any view on storage, but would specify how the data is stored, with TensorImpl handling only the actual memory allocation.

That's the reason why we discussed the idea of having an additionnal TensorView object, that would be a particular view on an existing Tensor, independently on the Tensor's implementation (TensorImpl). Tensor is garanteed to be contiguous in memory, while TensorView is not.

This is what I suggested:

classDiagram
  Data<|--Tensor
  Data<|--TensorView
  TensorView-->Tensor
  Tensor-->TensorImpl_CPU
  Tensor-->TensorImpl_CUDA
  Tensor-->TensorImpl_Eigen
  class Tensor{
     +vector<int> actualDims
     +TensorView getView()
  }
  class TensorView{
     +vector<int> dims
     +vector<int> strides
  }
  class TensorImpl_CPU{
     +T* data
  }
  class TensorImpl_CUDA{
     +T* deviceData
  }
  class TensorImpl_Eigen{
     +T* eigenData
  }

Your vision would be like this (correct it if I am wrong):

classDiagram
  Data<|--Tensor
  Tensor-->TensorImpl_CPU
  Tensor-->TensorImpl_CUDA
  Tensor-->TensorImpl_Eigen
  class Tensor{
     +vector<int> dims
     +vector<int> strides
     +Tensor getView()
  }
  class TensorImpl_CPU{
     +T* data
     +vector<int> actualDims
     +vector<int> actualStrides
  }
  class TensorImpl_CUDA{
     +T* deviceData
     +vector<int> actualDims
     +vector<int> actualStrides
  }
  class TensorImpl_Eigen{
     +T* eigenData
     +vector<int> actualDims
     +vector<int> actualStrides
  }

In this case, every operator must be able to handle any type of memory layout, which is problematic. In most operators implemenation, we make the basic assumption that data is contiguous in memory, with also the same layout according to the dimensions. While some implementation might accept other type of layout (e.g. CuDNN), there is usually a lot of restrictions. By having a different TensorView object, it is possible for the operators to specify if they accept a non-contiguous layout with strides. It also makes easy to insert operators in the computation graph to copy TensorView to actual Tensor if needed by the implementation.

And just to be clear: contiguous data means no padding!

added 19 commits

fc8dff77...29ba2cbf - 17 commits from branch main
3fae09e7 - [MERGE][main] updating branch with main
847f63bd - [WIP][NF] distinguishing different Tensor implementation state

Compare with previous version

added 2 commits

3268136d - [WIP][NF] distinguishing between having an impl and having an impl with data
3519cd7d - [BUILD] Adding stricter compilation options to unit tests

Compare with previous version

added 1 commit

695040aa - [FIX][TBV] fixing ComputeLayout

Compare with previous version

added 1 commit

4f573026 - [BUILD] temporarily remove strict compiler option to build unit tests

Compare with previous version

added 1 commit

05d95054 - [FIX] typo in CMakeLists.txt

Compare with previous version

added 1 commit

9815673b - [FIX][BUILD] sanitizing wrongly activated by default: fixed

Compare with previous version

added 34 commits

9815673b...8228ba63 - 30 commits from branch main
2cc13004 - [MERGE][master][WIP][NF] merging refactoring from master
365b8d59 - [MERGE][master]
676209c6 - [FIX] fixing after merge with main branch
60c2401b - [MERGE][main] updating branch with main

Compare with previous version

added 62 commits

60c2401b...d4f09c00 - 57 commits from branch main
dcc65f21 - [fix/build][FIX][BUILD][WIP] updating build related files
8f103239 - [FIX] fixing msvc compilation
260e3401 - [MERGE][main][WIP][NF] merging from main, still need to fix inconsistency between APIs
22c6276a - [MAJ] getIdx returns an index in number of elements
0c3c5027 - [MAJ] adjusting python binding to new interface