BTAS icon indicating copy to clipboard operation
BTAS copied to clipboard

TensorView returned by Tensor::slice seems to be indexed from the original 0

Open shiozaki opened this issue 11 years ago • 3 comments

The following code returns 1.0, which is wrong.

#include <iostream>
#include <btas/btas.h>
#include <btas/tensor.h>

using namespace std;
using namespace btas;

int main() {

  Range b(5);
  Tensor<double> a(b);

  a(0) = 1.0;
  a(1) = 2.0;
  a(2) = 3.0;
  a(3) = 4.0;
  a(4) = 5.0;

  auto low = {2};
  auto up  = {4};
  TensorView<double> tmp(a.range().slice(low, up), a.storage());

  cout << tmp(0) << endl;

  return 0;
}

shiozaki avatar Jul 16 '14 16:07 shiozaki

The following changes fix this. @emstoudenmire can you comment whether this is the right thing?

diff --git a/btas/ordinal.h b/btas/ordinal.h
index 83d0808..82d34ee 100644
--- a/btas/ordinal.h
+++ b/btas/ordinal.h
@@ -112,7 +112,7 @@ namespace btas {
         for(auto i = 0ul; i != end; ++i)
           o += *(std::begin(index) + i) * *(std::begin(this->stride_) + i);

-        return o - offset_;
+        return o + offset_;
       }

       /// Does ordinal value belong to this ordinal range?
diff --git a/btas/range.h b/btas/range.h
index 6d66e4a..b12245e 100644
--- a/btas/range.h
+++ b/btas/range.h
@@ -901,7 +901,7 @@ namespace btas {
       typename std::enable_if<btas::is_index<Index1>::value && btas::is_index<Index2>::value, RangeNd>::type
       slice(const Index1& lobound, const Index2& upbound) const
       {
-        return RangeNd(lobound, upbound, _Ordinal(this->lobound(), this->upbound(), this->ordinal().stride()));
+        return RangeNd(lobound, upbound, _Ordinal(lobound, upbound, this->ordinal().stride()));
       }

       /// Constructs a Range slice defined by a subrange for each dimension

shiozaki avatar Jul 16 '14 18:07 shiozaki

PS: seems this change breaks test_tensorview.C, so perhaps still something has to be done.

shiozaki avatar Jul 16 '14 18:07 shiozaki

I am still trying to wrap my head around this one regarding whether it's a problem with Range or with TensorView. At the moment I am thinking Range is ok and that the problem is that TensorView::operator()(x) plugs its argument directly into range_.ordinal(x) (apart from maybe converting x from an argument pack to a Range::index_type), which isn't the appropriate thing in this case.

emstoudenmire avatar Jul 18 '14 04:07 emstoudenmire