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

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

Open
shiozaki opened this issue Jul 16, 2014 · 3 comments
Labels

Comments

@shiozaki
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@emstoudenmire
Copy link
Contributor

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.

@shiozaki shiozaki added the bug label Jul 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants