Fix raised floating point exceptions for distributed tree

[aprokop]

No description provided.

[isteinbrecher]

Thanks for having a look at this!

I tried the current state of this PR 91d08e4 with the example provided in #1402 (comment) and it worked.

However, I also tried this commit with 4C, and there our distributed tree ArborX unit tests (https://github.com/4C-multiphysics/4C/blob/main/src/core/geometric_search/tests/4C_geometric_search_distributed_test.np3.cpp) fail with different search results than before. I have reduced this to a minimal example:

4C unit test minimal

// Standalone reproducer for
// src/core/geometric_search/tests/4C_geometric_search_distributed_test.np3.cpp
//
// Run with exactly three MPI ranks.

#include <ArborX.hpp>
#include <ArborX_DistributedTree.hpp>
#include <ArborX_KDOP.hpp>
#include <Kokkos_Core.hpp>
#include <mpi.h>

#include <array>
#include <iostream>
#include <map>
#include <string>
#include <utility>
#include <vector>


//
// Define structures similar to the ones used in 4C
//
using KDOP = ArborX::Experimental::KDOP<3, 26>;

struct FourC_BoundingVolume
{
  KDOP bounding_volume_;
  int gid_;
};

struct PrimitivesWrapper
{
  const std::vector<FourC_BoundingVolume>& bounding_volumes_;
};

struct PredicatesWrapper
{
  const std::vector<FourC_BoundingVolume>& bounding_volumes_;
};

struct CollisionSearchResult
{
  int lid_predicate;
  int gid_predicate;
  int gid_primitive;
  int pid_primitive;
};


namespace ArborX
{
  template <>
  struct AccessTraits<PrimitivesWrapper>
  {
    using memory_space = Kokkos::HostSpace;
    using size_type = Kokkos::HostSpace::size_type;

    KOKKOS_FUNCTION static size_type size(const PrimitivesWrapper& x)
    {
      return x.bounding_volumes_.size();
    }

    KOKKOS_FUNCTION static auto get(const PrimitivesWrapper& x, size_type i)
    {
      // Attach the bounding volumes GID
      return ArborX::PairValueIndex{
          .value = x.bounding_volumes_[i].bounding_volume_, .index = x.bounding_volumes_[i].gid_};
    }
  };

  template <>
  struct AccessTraits<PredicatesWrapper>
  {
    using memory_space = Kokkos::HostSpace;
    using size_type = Kokkos::HostSpace::size_type;

    KOKKOS_FUNCTION static size_type size(const PredicatesWrapper& x)
    {
      return x.bounding_volumes_.size();
    }

    KOKKOS_FUNCTION static auto get(const PredicatesWrapper& x, size_type i)
    {
      return ArborX::intersects(x.bounding_volumes_[i].bounding_volume_);
    }
  };
}  // namespace ArborX


//
// Boundig volumes for testing, similar to https://github.com/kokkos/ArborX/issues/867
//
std::array<KDOP, 3> make_volumes()
{
  std::array<KDOP, 3> volumes{};
  auto add = [&](int volume, float x, float y, float z)
  { ArborX::Details::expand(volumes[volume], ArborX::Point<3>{x, y, z}); };

  add(0, 0.0f, 0.0f, 0.0f);
  add(0, 1.0f, 1.0f, 0.0f);
  add(0, 0.5f, 0.0f, 0.0f);

  add(1, 1.0f, -1.0f, 0.0f);
  add(1, 1.0f, 0.25f, 0.0f);
  add(1, 0.75f, 0.0f, 0.0f);

  add(2, 0.5f, 0.25f, 0.0f);
  add(2, 0.8f, 0.25f, 0.0f);
  add(2, 0.75f, 0.125f, 0.0f);
  return volumes;
}

//
// Minimal direct-ArborX equivalent of 4C's global_collision_search().
//
std::vector<CollisionSearchResult> search(std::vector<FourC_BoundingVolume> const& primitives,
    std::vector<FourC_BoundingVolume> const& predicates, MPI_Comm comm)
{
  using ExecutionSpace = Kokkos::DefaultExecutionSpace;
  using MemorySpace = Kokkos::HostSpace;

  int rank = -1;
  MPI_Comm_rank(comm, &rank);
  ExecutionSpace execution_space;

  ArborX::DistributedTree tree(comm, execution_space, PrimitivesWrapper{primitives});

  Kokkos::View<Kokkos::pair<int, int>*, MemorySpace> hits("hits", 0);
  Kokkos::View<int*, MemorySpace> offsets("offsets", 0);
  auto callback = KOKKOS_LAMBDA(auto const&, auto const& primitive, auto const& out)
  {
    // Attach the primitive's GID and the rank of the primitive
    out(Kokkos::pair<int, int>{primitive.index, rank});
  };

  tree.query(execution_space, PredicatesWrapper{predicates}, callback, hits, offsets);

  std::vector<CollisionSearchResult> result;
  result.reserve(hits.size());
  for (std::size_t i = 0; i + 1 < offsets.size(); ++i)
    for (int j = offsets(i); j < offsets(i + 1); ++j)
      result.push_back({static_cast<int>(i), predicates[i].gid_, hits(j).first, hits(j).second});
  return result;
}

//
// Utility function to check the results.
//
using Key = std::pair<int, int>;
using Metadata = std::array<int, 2>;
using Expected = std::map<Key, Metadata>;
bool check(std::string const& name, std::vector<CollisionSearchResult> const& actual,
    Expected const& expected, int rank)
{
  bool ok = actual.size() == expected.size();
  if (!ok)
    std::cerr << "rank " << rank << ": " << name << ": expected " << expected.size()
              << " hits, got " << actual.size() << '\n';

  for (auto const& hit : actual)
  {
    auto const it = expected.find({hit.gid_predicate, hit.gid_primitive});
    if (it == expected.end() || it->second[0] != hit.lid_predicate ||
        it->second[1] != hit.pid_primitive)
    {
      ok = false;
      std::cerr << "rank " << rank << ": " << name << ": unexpected hit "
                << "{lid_predicate=" << hit.lid_predicate << ", gid_predicate=" << hit.gid_predicate
                << ", gid_primitive=" << hit.gid_primitive
                << ", pid_primitive=" << hit.pid_primitive << "}\n";
    }
  }
  return ok;
}

//
// Compare kDop vs. kDop on all ranks.
//
bool test_kdop_vs_kdop(MPI_Comm comm, int rank)
{
  auto const v = make_volumes();
  std::vector<FourC_BoundingVolume> primitives;
  std::vector<FourC_BoundingVolume> predicates;

  if (rank == 0)
  {
    primitives = {
        {.bounding_volume_ = v[0], .gid_ = 0},
        {.bounding_volume_ = v[2], .gid_ = 1},
        {.bounding_volume_ = v[1], .gid_ = 2},
    };
    predicates = {
        {.bounding_volume_ = v[0], .gid_ = 3},
        {.bounding_volume_ = v[2], .gid_ = 4},
        {.bounding_volume_ = v[1], .gid_ = 5},
    };
  }
  else if (rank == 1)
  {
    primitives = {
        {.bounding_volume_ = v[0], .gid_ = 6},
        {.bounding_volume_ = v[1], .gid_ = 7},
    };
    predicates = {
        {.bounding_volume_ = v[2], .gid_ = 8},
    };
  }
  else
  {
    primitives = {
        {.bounding_volume_ = v[0], .gid_ = 9},
        {.bounding_volume_ = v[2], .gid_ = 10},
    };
    predicates = {
        {.bounding_volume_ = v[1], .gid_ = 11},
    };
  }

  Expected expected;
  if (rank == 0)
    expected = {{{3, 0}, {0, 0}}, {{3, 1}, {0, 0}}, {{3, 6}, {0, 1}}, {{3, 9}, {0, 2}},
        {{3, 10}, {0, 2}}, {{4, 0}, {1, 0}}, {{4, 1}, {1, 0}}, {{4, 6}, {1, 1}}, {{4, 9}, {1, 2}},
        {{4, 10}, {1, 2}}, {{5, 2}, {2, 0}}, {{5, 7}, {2, 1}}};
  else if (rank == 1)
    expected = {
        {{8, 0}, {0, 0}}, {{8, 1}, {0, 0}}, {{8, 6}, {0, 1}}, {{8, 9}, {0, 2}}, {{8, 10}, {0, 2}}};
  else
    expected = {{{11, 2}, {0, 0}}, {{11, 7}, {0, 1}}};

  return check("KDOPVsKDOP", search(primitives, predicates, comm), expected, rank);
}

//
// Compare kDop vs. kDop where only rank 0 has primitives and only rank 1 has predicates.
//
bool test_mixed_empty_inputs(MPI_Comm comm, int rank)
{
  auto const v = make_volumes();
  std::vector<FourC_BoundingVolume> primitives;
  std::vector<FourC_BoundingVolume> predicates;
  if (rank == 0)
    primitives = {
        {.bounding_volume_ = v[0], .gid_ = 0},
        {.bounding_volume_ = v[2], .gid_ = 1},
        {.bounding_volume_ = v[1], .gid_ = 2},
    };
  else if (rank == 1)
    predicates = {
        {.bounding_volume_ = v[2], .gid_ = 3},
    };

  Expected expected;
  if (rank == 1) expected = {{{3, 0}, {0, 0}}, {{3, 1}, {0, 0}}};
  return check("MixedEmptyInputs", search(primitives, predicates, comm), expected, rank);
}

//
// Compare kDop vs. kDop where predicates and primitves on all ranks are empty.
//
bool test_all_empty(MPI_Comm comm, int rank)
{
  return check("AllEmpty", search({}, {}, comm), {}, rank);
}

int main(int argc, char* argv[])
{
  MPI_Init(&argc, &argv);
  int rank = -1;
  int size = 0;
  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
  MPI_Comm_size(MPI_COMM_WORLD, &size);

  if (size != 3)
  {
    if (rank == 0) std::cerr << "Run this with exactly 3 MPI ranks.\n";
    MPI_Finalize();
    return 2;
  }

  Kokkos::initialize(argc, argv);
  bool ok = test_kdop_vs_kdop(MPI_COMM_WORLD, rank);
  ok = test_mixed_empty_inputs(MPI_COMM_WORLD, rank) && ok;
  ok = test_all_empty(MPI_COMM_WORLD, rank) && ok;

  int local_ok = ok ? 1 : 0;
  int all_ok = 0;
  MPI_Allreduce(&local_ok, &all_ok, 1, MPI_INT, MPI_MIN, MPI_COMM_WORLD);
  if (rank == 0)
    std::cout << (all_ok ? "All distributed ArborX tests passed.\n"
                         : "At least one distributed ArborX test failed.\n");

  Kokkos::finalize();
  MPI_Finalize();
  return all_ok ? 0 : 1;
}

With the current master f804f07 I get the following output

All distributed ArborX tests passed.

With the current state of this PR 91d08e4 I get

rank 0: KDOPVsKDOP: expected 12 hits, got 0
rank 1: KDOPVsKDOP: expected 5 hits, got 0
rank 2: KDOPVsKDOP: expected 2 hits, got 0
rank 1: MixedEmptyInputs: expected 2 hits, got 0
At least one distributed ArborX test failed.

@maxfirmbach

[aprokop]

@isteinbrecher please give it another try

[Copilot]

🟡 Changes recommended

The new equality check breaks compilation for distributed trees using KDOP bounding volumes.

1 open issue

• Avoid unsupported KDOP equality requirement in DistributedTree · New

What changed in this PR

This PR addresses floating-point exceptions in distributed trees by excluding empty ranks from the top-level tree and handling nearest queries on empty trees.

Changes:

• Filter empty rank bounds before top-tree construction.
• Return empty results for nearest queries on empty trees.
• Re-enable and expand empty-tree query tests.

File	Description
test/​tstDistributedTreeNearest.cpp	Enables empty-tree tests and adds spatial-query coverage.
src/​distributed/​detail/​ArborX_DistributedTreeUtils.hpp	Adds empty-rank filtering.
src/​distributed/​detail/​ArborX_DistributedTreeNearest.hpp	Handles nearest queries on empty trees.
src/​distributed/​ArborX_DistributedTree.hpp	Builds the top tree from filtered bounds and adjusts a profiling label.

_{Review effort: Balanced}

[aprokop]

All the problems seem to be coming from the fact that I'm trying to detect whether the rank is empty by examining the bounding volume for the domain it produces. This is the only way to do it if we don't want to introduce extra communication.

I think this may be a wrong way to go as it introduces semantic changes. May just need to bite the bullet and do an extra round of communication.

[Copilot]

🟡 Changes recommended

An undeclared bottom_tree reference breaks compilation when GPU-aware MPI is enabled.

1 open issue

• Fix undeclared bottom_tree in GPU-aware MPI build · New

1 resolved since last review

• Avoid unsupported KDOP equality requirement in DistributedTree

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

Static review found no blocking issues in rank preservation, empty-tree handling, or the updated tests.

No open issues 🎉

1 resolved since last review

• Fix undeclared bottom_tree in GPU-aware MPI build

_{Review effort: Balanced}

[isteinbrecher]

@isteinbrecher please give it another try

I have tried all our applications with the commits 2701a52 and 09738a2 and the local tests all pass. To be sure I started a full pipeline in 4C with 09738a2.