Fix assertion in adjacent_difference

[yasahi-hpc]

Fixes kokkos#6890

This PR aims at adding assertions if adjacent_difference works on the identical source and destination views.
For this purpose, I added

1. New KOKKOS_EXPECT to make sure that source and destination views are not the same (expect_not_identical)
2. Add these assertions in adjacent_difference with Execution space
3. Added death tests in debug mode

When it comes to the team level, it is more complicated. Equality for different views may be satisfied on subviews having the extent of size 0 in some direction. This is happening in the test. I am not quite sure what is the correct way to avoid this situation.

[yasahi-hpc]

I have updated based on the comments from core.

In the new version,

1. New KOKKOS_EXPECT to make sure that source and destination views do not overlap
2. Calling find_first_of to check if there is an overlap
3. Death tests in debug mode excluded since it throws not aborts

[jbigot]

Maybe you could extract this to a Impl::expect_no_overlap similar to Impl::expect_valid_range, I guess it will be useful elsewhere

[yasahi-hpc]

That makes sense. In the end, I will implement Impl::expect_no_overlap and find_first_overlap_of with tests.

[jbigot]

I'm afraid find_first_of compares the content, not the adresses, I would check that if the iterators are compatble (i.e. is_valid_range(first_from, first_dest)), then either first_dest>last_from or first_dest+last_from-first_from<first_from

[yasahi-hpc]

That is true. I may need find_first_overlap_of to compare iterators.

[jbigot]

From my point of view, this is first a documentation issue, is the doc clear about the precondition? If so, the check in code is a nice thing to have, but not a requirement.

[yasahi-hpc]

From my point of view, this is first a documentation issue, is the doc clear about the precondition? If so, the check in code is a nice thing to have, but not a requirement.

I will implement as requested. At least, it would be a good practice

[yasahi-hpc]

I have updated based on the comments from core.

In the new version,

1. New KOKKOS_EXPECT to make sure that source and destination views do not overlap
2. Tests are performed only if iterators are construbile from other iterator.

Still the issue with death tests, if we add that I got the following failure.
Here is the DEATH tests.

  #if !defined(NDEBUG) || defined(KOKKOS_ENFORCE_CONTRACTS) || \
    defined(KOKKOS_ENABLE_DEBUG)
  {
    auto view_dest = view_from;
    EXPECT_DEATH(
      {KE::adjacent_difference(exespace(), view_from,
                               view_dest, args...);
       Kokkos::fence();
      },
      "Kokkos contract violation:.*");
  }
  {
    auto view_dest = view_from;
    EXPECT_DEATH(
      {KE::adjacent_difference("label", exespace(), view_from,
                               view_dest, args...);
       Kokkos::fence();
      },
      "Kokkos contract violation:.*");
  }
  Kokkos::fence();
  #endif

Errors

[WARNING] /gpfs/users/asahiy/work/Kokkos/kokkos/tpls/gtest/gtest/gtest-all.cc:9326:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 3 threads. See https://github.com/google/googletest/blob/master/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out.
/gpfs/users/asahiy/work/Kokkos/kokkos/algorithms/unit_tests/TestStdAlgorithmsAdjacentDifference.cpp:252: Failure
Death test: {KE::adjacent_difference("label", exespace(), view_from, view_dest, args...); Kokkos::fence(); }
    Result: threw an exception.
 Error msg:
[  DEATH   ] 
[  DEATH   ] /gpfs/users/asahiy/work/Kokkos/kokkos/algorithms/unit_tests/TestStdAlgorithmsAdjacentDifference.cpp:252:: Caught std::exception-derived exception escaping the death test statement. Exception message: cudaSetDevice(m_cudaDev) error( cudaErrorInitializationError): initialization error /gpfs/users/asahiy/work/Kokkos/kokkos/core/src/Cuda/Kokkos_Cuda_Instance.hpp:187
[  DEATH   ] 

[jbigot]

you can maybe add a else if constexpr that checks the opposite case that

[yasahi-hpc]

Do you mean that
there is a case that
std::is_constructible_v<IteratorType2, IteratorType1> is false but std::is_constructible_v<IteratorType1, IteratorType2> is true?

[jbigot]

Yes, while you can get a const iterator from a non const one, the opposite is not possible

[yasahi-hpc]

Sure. I will add that.

[jbigot]

looks great to me

[yasahi-hpc]

Thanks!