-
Notifications
You must be signed in to change notification settings - Fork 24
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
Tiles #922
Tiles #922
Changes from 7 commits
f3b9faf
96437f7
1efd27d
ffbc7fc
80c5501
34f4111
b47a9ad
b11781c
1792729
0cc4374
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,50 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#ifndef PHARE_PARTICLE_TILE_HPP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#define PHARE_PARTICLE_TILE_HPP | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include "core/utilities/box/box.hpp" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#include "core/data/tiles/tile_set.hpp" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
namespace PHARE::core | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
template<typename ParticleArray> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
struct ParticleTile : public Box<int, ParticleArray::dimension> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// *this: box constructed first | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ParticleArray domainParticles_{*this}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ParticleArray patchGhostParticles_{*this}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ParticleArray levelGhostParticles_{*this}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ParticleArray levelGhostParticlesNew_{*this}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ParticleArray levelGhostParticlesOld_{*this}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
template<typename ParticleArray> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
class ParticleTileSet : public TileSet<ParticleTile<ParticleArray>> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
public: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
static auto constexpr dimension = ParticleArray::dimension; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto select_particles(Box<int, dimension> const& from) const | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ParticleArray selected; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto overlaped_tiles = export_overlaped_with(from); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
for (auto const& [complete, tile] : overlaped_tiles) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (complete) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
selected.push_back(tile.domainParticles_); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
auto intersection = tile * from; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tile.domainParticles.export_to(*intersection, selected); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return selected; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+28
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix critical issues in select_particles method. Several issues need to be addressed:
Here's the corrected implementation: auto select_particles(Box<int, dimension> const& from) const
{
ParticleArray selected;
- auto overlaped_tiles = export_overlaped_with(from);
- for (auto const& [complete, tile] : overlaped_tiles)
+ if (from.dimension() != dimension)
+ throw std::invalid_argument("Box dimension mismatch");
+
+ auto overlapped_tiles = export_overlapped_with(from);
+ for (auto const& [complete, tile] : overlapped_tiles)
{
if (complete)
{
selected.push_back(tile.domainParticles_);
}
else
{
auto intersection = tile * from;
- tile.domainParticles.export_to(*intersection, selected);
+ tile.domainParticles_.export_to(*intersection, selected);
}
}
return selected;
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} // namespace PHARE::core | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
#endif |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,199 @@ | ||||||||||||||||||||||||||||||||||||||||||
#ifndef PHARE_TILE_SET_HPP | ||||||||||||||||||||||||||||||||||||||||||
#define PHARE_TILE_SET_HPP | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#include "core/utilities/box/box.hpp" | ||||||||||||||||||||||||||||||||||||||||||
#include "core/utilities/types.hpp" | ||||||||||||||||||||||||||||||||||||||||||
#include "core/def.hpp" | ||||||||||||||||||||||||||||||||||||||||||
#include "core/data/ndarray/ndarray_vector.hpp" | ||||||||||||||||||||||||||||||||||||||||||
#include "core/utilities/span.hpp" | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#include <array> | ||||||||||||||||||||||||||||||||||||||||||
#include <utility> | ||||||||||||||||||||||||||||||||||||||||||
#include <string> | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
namespace PHARE::core | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
template<typename Tile> | ||||||||||||||||||||||||||||||||||||||||||
class TileSetView | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
public: | ||||||||||||||||||||||||||||||||||||||||||
static auto constexpr dimension = Tile::dimension; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
TileSetView(Box<int, dimension> const& box, std::array<std::size_t, dimension> const& tile_size, | ||||||||||||||||||||||||||||||||||||||||||
std::array<int, dimension> const& shape, Tile** tiles, std::size_t tile_nbr, | ||||||||||||||||||||||||||||||||||||||||||
int** cells, std::size_t nbr_cells) | ||||||||||||||||||||||||||||||||||||||||||
: box_{box} | ||||||||||||||||||||||||||||||||||||||||||
, tile_size_{tile_size} | ||||||||||||||||||||||||||||||||||||||||||
, shape_{shape} | ||||||||||||||||||||||||||||||||||||||||||
, tiles_{tiles, tile_nbr} | ||||||||||||||||||||||||||||||||||||||||||
, cells_{cells, nbr_cells} | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
private: | ||||||||||||||||||||||||||||||||||||||||||
Box<int, dimension> const box_; | ||||||||||||||||||||||||||||||||||||||||||
std::array<std::size_t, dimension> tile_size_; | ||||||||||||||||||||||||||||||||||||||||||
std::array<int, dimension> shape_; | ||||||||||||||||||||||||||||||||||||||||||
Span<Tile*> tiles_; | ||||||||||||||||||||||||||||||||||||||||||
NdArrayView<dimension, Tile*> cells_; | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
template<typename Tile> | ||||||||||||||||||||||||||||||||||||||||||
class TileSet | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
public: | ||||||||||||||||||||||||||||||||||||||||||
static auto constexpr dimension = Tile::dimension; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
TileSet(Box<int, dimension> const& box, std::array<std::size_t, dimension> const& tile_size) | ||||||||||||||||||||||||||||||||||||||||||
: box_{box} | ||||||||||||||||||||||||||||||||||||||||||
, tile_size_{tile_size} | ||||||||||||||||||||||||||||||||||||||||||
, shape_{[&]() { | ||||||||||||||||||||||||||||||||||||||||||
std::array<int, dimension> s; | ||||||||||||||||||||||||||||||||||||||||||
auto bs = box.shape(); | ||||||||||||||||||||||||||||||||||||||||||
for (auto i = 0u; i < dimension; ++i) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto const div = (bs[i] + tile_size_[i] - 1) / tile_size_[i]; | ||||||||||||||||||||||||||||||||||||||||||
s[i] = div; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
return s; | ||||||||||||||||||||||||||||||||||||||||||
}()} | ||||||||||||||||||||||||||||||||||||||||||
, tiles_(product(shape_)) | ||||||||||||||||||||||||||||||||||||||||||
, cells_{box.shape().template toArray<std::uint32_t>()} | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
consistent_tile_size_(); | ||||||||||||||||||||||||||||||||||||||||||
make_tiles_(); | ||||||||||||||||||||||||||||||||||||||||||
tag_cells_(); | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto export_overlaped_with(Box<int, dimension> const& box) const | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
std::vector<std::pair<bool, Tile>> overlaped; | ||||||||||||||||||||||||||||||||||||||||||
for (auto const& tile : tiles_) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto overlap = box * Box<int, dimension>{tile.lower, tile.upper}; | ||||||||||||||||||||||||||||||||||||||||||
if (overlap) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto complete_overlap = (*overlap).size() == tile.size(); | ||||||||||||||||||||||||||||||||||||||||||
overlaped.emplace_back(complete_overlap, tile); | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
return overlaped; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+106
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo in method and variable names: 'overlaped' should be 'overlapped' There is a typo in the method name Apply this diff to correct the typos: - NO_DISCARD auto export_overlaped_with(Box<int, dimension> const& box) const
+ NO_DISCARD auto export_overlapped_with(Box<int, dimension> const& box) const
- std::vector<std::pair<bool, Tile>> overlaped;
+ std::vector<std::pair<bool, Tile>> overlapped;
🛠️ Refactor suggestion Avoid unnecessary copying in 'export_overlapped_with' by returning pointers to tiles Currently, Apply this diff to modify the vector to store pointers to - std::vector<std::pair<bool, Tile>> overlapped;
+ std::vector<std::pair<bool, Tile const*>> overlapped;
for (auto const& tile : tiles_)
{
auto overlap = box * Box<int, dimension>{tile.lower, tile.upper};
if (overlap)
{
auto complete_overlap = (*overlap).size() == tile.size();
- overlapped.emplace_back(complete_overlap, tile);
+ overlapped.emplace_back(complete_overlap, &tile);
}
}
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto shape() const { return shape_; } | ||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto size() const { return tiles_.size(); } | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto begin() { return tiles_.begin(); } | ||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto begin() const { return tiles_.begin(); } | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto end() { return tiles_.end(); } | ||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto end() const { return tiles_.end(); } | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto& operator[](std::size_t i) { return tiles_[i]; } | ||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto const& operator[](std::size_t i) const { return tiles_[i]; } | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
template<typename... Index> | ||||||||||||||||||||||||||||||||||||||||||
NO_DISCARD auto& at(Index... indexes) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
return cells_(indexes...); | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
auto make_view() | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
return TileSetView<Tile>{box_, tile_size_, shape_, tiles_.data(), | ||||||||||||||||||||||||||||||||||||||||||
tiles_.size(), cells_.data(), cells_.size()}; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
private: | ||||||||||||||||||||||||||||||||||||||||||
void consistent_tile_size_() const | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
for (auto idim = 0u; idim < dimension; ++idim) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
if (box_.shape()[idim] < tile_size_[idim]) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
throw std::runtime_error("tile size larger than box size in dimension " | ||||||||||||||||||||||||||||||||||||||||||
+ std::to_string(idim)); | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+149
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add check to prevent zero Currently, the Apply this diff to add a check for zero void consistent_tile_size_() const
{
for (auto idim = 0u; idim < dimension; ++idim)
{
+ if (tile_size_[idim] == 0)
+ {
+ throw std::runtime_error("tile size cannot be zero in dimension " + std::to_string(idim));
+ }
if (box_.shape()[idim] < tile_size_[idim])
{
throw std::runtime_error("tile size larger than box size in dimension "
+ std::to_string(idim));
}
}
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
void make_tiles_() | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto const size_me = [&](auto dim, auto idx) { | ||||||||||||||||||||||||||||||||||||||||||
if (idx == shape_[dim] - 1) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto const remain = box_.shape()[dim] % tile_size_[dim]; | ||||||||||||||||||||||||||||||||||||||||||
return (remain == 0) ? tile_size_[dim] : remain; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||||||||||
return tile_size_[dim]; | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
for (auto ix = 0u; ix < shape_[0]; ++ix) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
if constexpr (dimension == 1) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
// -1 because upper is included | ||||||||||||||||||||||||||||||||||||||||||
tiles_[ix].lower[0] = box_.lower[0] + ix * tile_size_[0]; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[ix].upper[0] = tiles_[ix].lower[0] + size_me(0, ix) - 1; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
for (auto iy = 0u; iy < shape_[1]; ++iy) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
if constexpr (dimension == 2) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto const i = ix * shape_[1] + iy; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].lower[0] = box_.lower[0] + ix * tile_size_[0]; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].upper[0] = tiles_[i].lower[0] + size_me(0, ix) - 1; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].lower[1] = box_.lower[1] + iy * tile_size_[1]; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].upper[1] = tiles_[i].lower[1] + size_me(1, iy) - 1; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
else | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
for (auto iz = 0u; iz < shape_[2]; ++iz) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
auto const i = ix * shape_[1] * shape_[2] + shape_[2] * iy + iz; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].lower[0] = box_.lower[0] + ix * tile_size_[0]; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].upper[0] = tiles_[i].lower[0] + size_me(0, ix) - 1; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].lower[1] = box_.lower[1] + iy * tile_size_[1]; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].upper[1] = tiles_[i].lower[1] + size_me(1, iy) - 1; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].lower[2] = box_.lower[2] + iz * tile_size_[2]; | ||||||||||||||||||||||||||||||||||||||||||
tiles_[i].upper[2] = tiles_[i].lower[2] + size_me(2, iz) - 1; | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+159
to
+207
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Refactor dimension-specific tile creation logic. The Consider this refactoring approach: private:
template<std::size_t Dim>
void make_tiles_impl_();
std::size_t calculate_tile_index(std::array<std::uint32_t, dimension> const& coords) const {
if constexpr (dimension == 1) {
return coords[0];
} else if constexpr (dimension == 2) {
return coords[0] * shape_[1] + coords[1];
} else {
return coords[0] * shape_[1] * shape_[2] + shape_[2] * coords[1] + coords[2];
}
}
void make_tiles_() {
if constexpr (dimension == 1) {
make_tiles_impl_<1>();
} else if constexpr (dimension == 2) {
make_tiles_impl_<2>();
} else {
make_tiles_impl_<3>();
}
} |
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
//! store the pointer to the tile associated with each cell | ||||||||||||||||||||||||||||||||||||||||||
void tag_cells_() | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
for (auto& tile : tiles_) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
for (auto const& cell : tile) | ||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||
cells_(cell) = &tile; | ||||||||||||||||||||||||||||||||||||||||||
Check warning Code scanning / CodeQL Local variable address stored in non-local memory Warning
A stack address (
source Error loading related location Loading |
||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
Box<int, dimension> box_; | ||||||||||||||||||||||||||||||||||||||||||
std::array<std::size_t, dimension> tile_size_; | ||||||||||||||||||||||||||||||||||||||||||
std::array<int, dimension> shape_; | ||||||||||||||||||||||||||||||||||||||||||
std::vector<Tile> tiles_; | ||||||||||||||||||||||||||||||||||||||||||
NdArrayVector<dimension, Tile*> cells_; | ||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
} // namespace PHARE::core | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
#include "gmock/gmock.h" | ||
#include "gtest/gtest.h" | ||
|
||
#include "core/data/particles/particle_array.hpp" | ||
#include "core/data/particles/particle_tile.hpp" | ||
#include "core/data/tiles/tile_set.hpp" | ||
|
||
using namespace PHARE::core; | ||
|
||
template<std::size_t dimension> | ||
using ParticleTile_t = ParticleTile<ParticleArray<dimension>>; | ||
|
||
template<std::size_t dimension> | ||
using TileSet_t = TileSet<ParticleTile_t<dimension>>; | ||
|
||
using DimParticleTiles = testing::Types<ParticleTile_t<1>, ParticleTile_t<2>, ParticleTile_t<3>>; | ||
|
||
template<typename ParticleTile> | ||
class ParticleTileTest : public ::testing::Test | ||
{ | ||
protected: | ||
ParticleTileTest() {} | ||
}; | ||
|
||
TYPED_TEST_SUITE(ParticleTileTest, DimParticleTiles); | ||
Comment on lines
+18
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance test fixture with common test scenarios. The current fixture is minimal. Consider adding:
Example enhancement: template<typename ParticleTile>
class ParticleTileTest : public ::testing::Test {
protected:
static constexpr auto dim = ParticleTile::dimension;
Box<int, dim> defaultBox{ConstArray<int, dim>(0), ConstArray<int, dim>(50)};
ConstArray<std::size_t, dim> defaultTileSize{4};
void SetUp() override {
// Setup common test state
}
void TearDown() override {
// Cleanup if needed
}
// Helper methods for common validations
void validateTileProperties(const ParticleTile& tile) {
// Add validation logic
}
}; |
||
|
||
TYPED_TEST(ParticleTileTest, constructs) | ||
Check notice Code scanning / CodeQL Unused static variable Note test
Static variable gtest_ParticleTileTest_constructs_registered_ is never read.
|
||
{ | ||
constexpr auto dim = TypeParam::dimension; | ||
Box<int, dim> box{ConstArray<int, dim>(0), ConstArray<int, dim>(50)}; | ||
auto const tile_size = PHARE::core::ConstArray<std::size_t, dim>(4); | ||
TileSet<TypeParam> particleTileSet{box, tile_size}; | ||
} | ||
Comment on lines
+27
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add comprehensive test coverage. The current test only verifies basic construction. Consider adding tests for:
Example additional test: TYPED_TEST(ParticleTileTest, handlesInvalidTileSize)
{
constexpr auto dim = TypeParam::dimension;
Box<int, dim> box{ConstArray<int, dim>(0), ConstArray<int, dim>(50)};
auto const invalid_tile_size = PHARE::core::ConstArray<std::size_t, dim>(0);
EXPECT_THROW({
TileSet<TypeParam> particleTileSet{box, invalid_tile_size};
}, std::invalid_argument);
}
TYPED_TEST(ParticleTileTest, correctlyPartitionsTiles)
{
constexpr auto dim = TypeParam::dimension;
Box<int, dim> box{ConstArray<int, dim>(0), ConstArray<int, dim>(10)};
auto const tile_size = PHARE::core::ConstArray<std::size_t, dim>(2);
TileSet<TypeParam> particleTileSet{box, tile_size};
// Verify number of tiles and their positions
// Add assertions here
} |
||
|
||
int main(int argc, char** argv) | ||
{ | ||
::testing::InitGoogleTest(&argc, argv); | ||
|
||
return RUN_ALL_TESTS(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
cmake_minimum_required (VERSION 3.20.1) | ||
|
||
project(test-tile) | ||
|
||
set(SOURCES test_tile.cpp) | ||
|
||
add_executable(${PROJECT_NAME} ${SOURCES}) | ||
|
||
target_include_directories(${PROJECT_NAME} PRIVATE | ||
${GTEST_INCLUDE_DIRS} | ||
) | ||
|
||
target_link_libraries(${PROJECT_NAME} PRIVATE | ||
phare_core | ||
${GTEST_LIBS}) | ||
|
||
add_no_mpi_phare_test(${PROJECT_NAME} ${CMAKE_CURRENT_BINARY_DIR}) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add documentation and consider class semantics.
Several improvements are recommended:
Here's a suggested improvement: