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

Representing ToTs with an independent index screened out #248

Open
ryanmrichard opened this issue Jan 13, 2021 · 2 comments
Open

Representing ToTs with an independent index screened out #248

ryanmrichard opened this issue Jan 13, 2021 · 2 comments

Comments

@ryanmrichard
Copy link
Contributor

Not sure how to describe this/where to attribute blame. Also, this isn't a pressing issue as I think it's unlikely to arise outside of unit testing.

This issue concerns how ToTs are represented when an entire independent index is screened out. This is probably easiest to explain with an example.

Consider the following two ToTs as printed by TA. The first ToT:

0: [ [0,0], [1,2] ) {
  [0,0]:[ [0], [2] ) { 1 2 }
  [0,1]:[ [0], [2] ) { 7 8 }
}
2: [ [1,0], [2,2] ) {
  [1,0]:[ [0], [2] ) { 0 2 }
  [1,1]:[ [0], [2] ) { 6 8 }
}
7: [ [3,2], [4,3] ) {
  [3,2]:[ [0], [2] ) { 12 13 }
}

This tensor is made with make_array and a lambda. As you can see when TA hits the screened out independent indices (1, 3,4,5, and 6) it implicitly stores the tiles associated with those indices. Now consider another tensor:

0: [ [0,0], [1,2] ) {
  [0,0]:[ [0], [2] ) { 1 2 }
  [0,1]:[ [0], [2] ) { 7 8 }
}
1: [ [0,2], [1,3] ) {
  [0,2]:[ [], [] ) { }
}
2: [ [1,0], [2,2] ) {
  [1,0]:[ [0], [2] ) { 0 2 }
  [1,1]:[ [0], [2] ) { 6 8 }
}
3: [ [1,2], [2,3] ) {
  [1,2]:[ [], [] ) { }
}
4: [ [2,0], [3,2] ) {
  [2,0]:[ [], [] ) { }
  [2,1]:[ [], [] ) { }
}
5: [ [2,2], [3,3] ) {
  [2,2]:[ [], [] ) { }
}
6: [ [3,0], [4,2] ) {
  [3,0]:[ [], [] ) { }
  [3,1]:[ [], [] ) { }
}
7: [ [3,2], [4,3] ) {
  [3,2]:[ [0], [2] ) { 12 13 }
}

This tensor is created with an initializer list like:

using inner_tile = TA::Tensor<double>;
inner_tile corr00{TA::Range{2},{1, 2}};
inner_tile corr01{TA::Range{2}, {7, 8}};
inner_tile corr10{TA::Range{2}, {0, 2}};
inner_tile corr11{TA::Range{2},{6, 8}};
inner_tile corr32{TA::Range{2}, {12, 13}};
inner_tile zero;
TA::detail::matrix_il<inner_tile> corr_il{
    {corr00, corr01, zero},
    {corr10, corr11, zero},
    {zero, zero, zero},
    {zero, zero, corr32}
};

Note you have to put the zero place-holder inner tensors there as all elements of the outer tensors need to be initialized. Also note you can't do something like: inner_tile zero{TA::Range{1}, {0}} because that's a different shape (you're now saying the independent index maps to a single dependent index, whose value is zero as opposed to mapping to nothing). The intent is for these tensors to be the same, but attempting to subtract them gives an error:

TiledArray/tensor/kernels.h:422: void TiledArray::detail::tensor_init(Op&&, TR&, const Ts& ...) [with Op = TiledArray::Tensor< <template-parameter-1-1>, <template-parameter-1-2> >::neg() const [with T = TiledArray::Tensor<double, Eigen::aligned_allocator<double> >; A = Eigen::aligned_allocator<TiledArray::Tensor<double, Eigen::aligned_allocator<double> > >; TiledArray::Tensor< <template-parameter-1-1>, <template-parameter-1-2> >::Tensor_ = TiledArray::Tensor<TiledArray::Tensor<double, Eigen::aligned_allocator<double> >, Eigen::aligned_allocator<TiledArray::Tensor<double, Eigen::aligned_allocator<double> > > >]::<lambda(TiledArray::Tensor<TiledArray::Tensor<double, Eigen::aligned_allocator<double> >, Eigen::aligned_allocator<TiledArray::Tensor<double, Eigen::aligned_allocator<double> > > >::numeric_type)>&; TR = TiledArray::Tensor<double, Eigen::aligned_allocator<double> >; Ts = {TiledArray::Tensor<double, Eigen::aligned_allocator<double> >}; typename std::enable_if<(TiledArray::detail::is_tensor<T2, Ts ...>::value && TiledArray::detail::is_contiguous_tensor<T2, Ts ...>::value)>::type* <anonymous> = 0]: Assertion `!empty(result, tensors...)' failed

So I don't know whether to call this an issue with representation (namely how to specify the initializer list for this scenario), an issue with the SparseArray ctor (should it have actually screened out the tile so it's implicitly stored), or an issue with operations like subtraction which don't know what to do with the empty inner tensor.

ryanmrichard added a commit to NWChemEx/Chemist that referenced this issue Jan 13, 2021
ryanmrichard added a commit to NWChemEx/Chemist that referenced this issue Jan 13, 2021
* Locks TA version and fixes initialization error

* adds Jeff's other fix

* comment out tests affected by ValeevGroup/tiledarray#248
@evaleev
Copy link
Member

evaleev commented Jan 13, 2021

  • initializer list by definition cannot specify sparse data. So every tile will be created and set. In fact the existing DistArray ctors that take initializer lists create arrays with "dense" shapes (even if the array type is sparse; see https://github.com/ValeevGroup/tiledarray/blob/master/src/TiledArray/util/initializer_list.h#L321), thus every tile must be set. These DistArray ctors would need to be modified to produce sparse arrays. In my opinion initializer lists are not well suited as input for sparse arrays.
  • what you are trying to make is a sparse array, with some tiles missing ("zero"), so you need to specify the shape explicitly. It is possible to fill tiles with zeros and then array.truncate(), but that is not efficient. Also, truncate needs to know how to compute tile norms, and I'm not sure it knows how to deal with ToT or null tiles.

@ryanmrichard
Copy link
Contributor Author

Makes sense. I'd think that the truncate option is the way to go for my scenario; however, as you guessed it can't compute the norm (presumably on account of the empty tiles, but I admittedly didn't investigate any further).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants