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

Flatten data structures in geometry #2313

Closed
wants to merge 25 commits into from

Conversation

SarahRo
Copy link
Contributor

@SarahRo SarahRo commented Aug 4, 2022

Replace std::array<std::array<double, 3>, 2> with std::array<double, 6>, and std::vector<std::array<int, 2>> with std::vector<int>.

Avoid copying data in some places where it was unnecessary.

@SarahRo SarahRo requested a review from jorgensd August 4, 2022 11:20
cpp/dolfinx/geometry/BoundingBoxTree.h Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.h Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Show resolved Hide resolved
@jorgensd jorgensd marked this pull request as ready for review August 10, 2022 09:07
Copy link
Member

@garth-wells garth-wells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the changes from std::array<X, 6> to std::span<const X, 6> in interfaces really needed? The cost of a copy should be more-or-less trivial. Is there some performance data?

cpp/dolfinx/geometry/BoundingBoxTree.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.cpp Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.h Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.h Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/BoundingBoxTree.h Outdated Show resolved Hide resolved
cpp/dolfinx/geometry/utils.cpp Outdated Show resolved Hide resolved
@garth-wells garth-wells added the enhancement New feature or request label Aug 23, 2022
@jorgensd
Copy link
Member

jorgensd commented Sep 7, 2022

This PR

Build tree (mus): 8889 11042 9456
Coll tree (mus): 555966 580818  566159
Build tree (mus): 8869 10062 9247
Coll tree (mus): 559656 581611  568570
Build tree (mus): 10099 11832 10811
Coll tree (mus): 621510 720529  663874
Build tree (mus): 9307 11173 9905
Coll tree (mus): 574994 626194  597880

Main

Build tree (mus): 9111 12733 10232
Coll tree (mus): 829225 955935  864322
Build tree (mus): 9145 11842 9804
Coll tree (mus): 830766 866580  840812
Build tree (mus): 9040 10724 9700
Coll tree (mus): 830286 906452  851835

with the following test:

#include <basix/finite-element.h>
#include <chrono>
#include <dolfinx.h>
#include <dolfinx/fem/FiniteElement.h>
#include <dolfinx/geometry/utils.h>

using namespace dolfinx;
int main(int argc, char* argv[])
{
  dolfinx::init_logging(argc, argv);
  MPI_Init(&argc, &argv);

  {
    // Create mesh and function space
    auto part = mesh::create_cell_partitioner(mesh::GhostMode::shared_facet);
    auto mesh = std::make_shared<mesh::Mesh>(
        mesh::create_box(MPI_COMM_WORLD, {{{0.0, 0.0, 0.0}, {1.0, 1.0, 1.0}}},
                         {15, 15, 15}, mesh::CellType::tetrahedron, part));

    const std::size_t num_runs = 10;
    std::vector<int> build_time(num_runs);
    std::vector<int> coll_time(num_runs);
    for (std::size_t i = 0; i < num_runs; ++i)
    {
      auto begint = std::chrono::high_resolution_clock::now();
      geometry::BoundingBoxTree tree(*mesh, mesh->topology().dim());
      auto endt = std::chrono::high_resolution_clock::now();
      auto durationt
          = std::chrono::duration_cast<std::chrono::microseconds>(endt - begint)
                .count();
      build_time[i] = durationt;
      basix::FiniteElement b_el = basix::create_element(
          basix::element::family::P, basix::cell::type::tetrahedron, 3,
          basix::element::lagrange_variant::equispaced);
      fem::FiniteElement el(b_el, 1);
      std::size_t num_cells_local
          = mesh->topology().index_map(mesh->topology().dim())->size_local();
      std::vector<std::int32_t> cells(num_cells_local);
      std::iota(cells.begin(), cells.end(), 0);
      std::vector<double> coords = fem::interpolation_coords(el, *mesh, cells);

      auto begin = std::chrono::high_resolution_clock::now();
      graph::AdjacencyList<std::int32_t> cols
          = dolfinx::geometry::compute_collisions(
              tree, std::span<const double>(coords));
      auto end = std::chrono::high_resolution_clock::now();

      auto duration
          = std::chrono::duration_cast<std::chrono::microseconds>(end - begin)
                .count();
      coll_time[i] = duration;
    }

    {
      auto min_time = std::min_element(build_time.begin(), build_time.end());
      auto max_time = std::max_element(build_time.begin(), build_time.end());
      auto avg_time = std::accumulate(build_time.begin(), build_time.end(), 0);
      std::cout << "Build tree (mus): " << *min_time << " " << *max_time << " "
                << avg_time / num_runs << std::endl;
    }

    {
      auto min_time = std::min_element(coll_time.begin(), coll_time.end());
      auto max_time = std::max_element(coll_time.begin(), coll_time.end());
      auto avg_time = std::accumulate(coll_time.begin(), coll_time.end(), 0);
      std::cout << "Coll tree (mus): " << *min_time << " " << *max_time << " "
                << " " << avg_time / num_runs << std::endl;
    }
  }

  MPI_Finalize();

  return 0;
}

@jorgensd
Copy link
Member

jorgensd commented Sep 7, 2022

@garth-wells I've now considered all your comments.
I've asked @IgorBaratta to verify the perfomance results on CSD3 (or a bigger system than a laptop).

@jorgensd
Copy link
Member

jorgensd commented Sep 9, 2022

Superseeded by #2359

@jorgensd jorgensd closed this Sep 9, 2022
@jorgensd jorgensd deleted the sarah/geometry-flatten-data-structures branch May 22, 2024 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants