Tpetra: offsetView of offsetView buggy

[jthies]

@trilinos/tpetra

[jthies]

If you create a simple Tpetra vector (with LocallyReplicated map in the attached example) and first create an offsetView V1, and then an offsetView V2 of V1, the offsets should be summed, but this doesn't seem the case since I updated from 11.12.1 to 12.2.1.
tpetra_bug.zip

[jthies]

sorry for the messy bug report, I'm new to github...

[mhoemmen]

Hi @jthies ! I'm not getting e-mail notifications for Tpetra for some reason -- I have to figure out what's going on -- but thanks for reporting!

[jhux2]

Mark,
I added the @mention to Jonas' first comment, since it was originally empty. I wasn't sure if it if would alert you -- apparently it did not.

Jonathan

[jthies]

could someone fix this minor issue? It's preventing me from porting Software to the latest Trilinos Versions (or has it been fixed already? I didn't try it anymore)

[mhoemmen]

@jthies Hi Jonas -- I'll work on adding your test to the repo. I've been traveling a lot and it's been hard for me to keep track of all the things to do. Thanks for explaining the urgency for us.

[mhoemmen]

I'm calling this a "task," but please note that it does relate to known DualView issues, like #364.

[mhoemmen]

@jthies Hi Jonas! You might be pleased, annoyed, or both to note that I've finally gotten around to fixing this. Thanks for catching this and for providing a test! I modified it slightly so that it works as an automated test. I'm running the check-in tests now and am about to push.

[mhoemmen]

@jthies I pushed a fix for this yesterday to the develop branch of Trilinos. Sorry it took us so long!

[jthies]

Hi Mark,

thanks, I’ll try it out soon and let you know if all our tests are running through!

Jonas

[mhoemmen]

thanks, I’ll try it out soon and let you know if all our tests are running through!

Thanks for still talking to me after it took so long for us to fix this silly thing! I'm not even terribly pleased with the fix, but as long as you don't exercise the weird use case of "recover the original column Map view of a domain Map view of a column Map (Multi)Vector, also with a nonzero row offset," your code should work fine.

[jthies]

Hi Mark,

sure, thank you for still listening to These nasty technical issues :)

The Situation I reported here seems to be fixed, but we still have some failing tests concerning multiple views. It might be that I didn't adapt everywhere to the changed "view semantics" though.

If you are interested in running our tests, you could check out https://bitbucket.org/essex/phist.git (branch tpetra_update).

Use cmake -DPHIST_KERNEL_LIB=tpetra, make and run the Driver phist-1.1.0-kernels-test-

Or you could add similar tests to your Framework, basically what we do is also run all our tests once with standard objects and once with funny views.

[mhoemmen]

@jthies wrote:

sure, thank you for still listening to These nasty technical issues :)

This is super important, and I appreciate your patience :-)

. . . basically what we do is also run all our tests once with standard objects and once with funny views.

Could you explain what you mean by "standard objects" vs. "funny views"? Are "funny views" the result of calling offsetView* on a Tpetra::MultiVector, with a nonzero offset?

[jthies]

The operations I consider are e.g. C=V^TW, W=V*C with C a "small dense" and V,W "tall skinny" matrices. In Tpetra both are represented as MultiVectors with a local or distributed map, respectively. For the V/W Kind, I want to allow views with a column offset only (subView with a Range1D arg). For the C-type, I also want to use views with a row and a column offset, so an "offsetView" of a "subview", again for contiguous rows and columns. I certainly don't care what the map of such an object Looks like, I just want to use it for local stuff anyway.

As to why we test this stuff with "funny views" (meaning e.g. V has a col offset, C a row and a col offset, and then compute C=V^TV): because the viewing leads to unaligned memory accesses and may break the kernel if it uses e.g. AVX, a mistake that we encounter quite often.

The Errors I see with the "develop" branch of Tpetra seem to occur mostly when putting view objects into tests that create more views of them and use thm to perform some operations, so the nesting of subViews and offsetViews, and mostly when "C-type" matrices are involved (offsetViews, that is). This all works in Trilinos 11.12.1, so it broke somewhere during the recent refectorings.

[mhoemmen]

@jthies wrote:

I certainly don't care what the map of such an object Looks like, I just want to use it for local stuff anyway.

I think you would be pleased with Kokkos::View. It's entirely local (no MPI), uses first-touch allocation by default for host memory, and can use NVIDIA GPU memory as well. View works almost like Matlab arrays; the only function you need to remember is Kokkos::subview. You can also choose the layout (e.g., column or row major; default depends on where you allocate). For example:

using Kokkos::ALL;
using Kokkos::LayoutLeft;
using Kokkos::pair;
using Kokkos::subview;
using Kokkos::View;

// Allocate a 2-D array, stored in column-major order.
View<double**, LayoutLeft> W ("W", numRows, numCols);

// Take a subview of W, rows 3, ..., 99, and columns 4, ..., 7.
// Compare to Matlab's W(3:99, 4:7).
auto W1 = subview (W, pair<int, int> (3, 100), pair<int, int> (4, 8));

// Take a subview of W, all rows, columns 4, ..., 7.
// Compare to Matlab's W(:, 4:7).
auto W2 = subview (W, ALL (), pair<int, int> (4, 8));

// Take a subview of W, rows 3, ..., 99, all columns.
// Compare to Matlab's W(3:99, :).
auto W3 = subview (W, pair<int, int> (3, 100), ALL ());

We're working on BLAS wrappers too, but it wouldn't be hard for you to write them, since the only function you need really is _GEMM. Our wrappers will take Views with any stride or layout, with special cases that use optimized BLAS when available.

[mhoemmen]

@jthies wrote:

This all works in Trilinos 11.12.1, so it broke somewhere during the recent refactorings.

See e.g., tpetra/ReleaseNotes.txt for 11.14. Could I trouble you perhaps to extract a minimal Tpetra use case that didn't break before, but breaks now? Thanks :-)