From 32c11613ee499b609999e6e54bcdbf24f94c5627 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 13:07:31 -0600 Subject: [PATCH 01/14] Remove undefined constructors --- src/interface/meshblock_data.hpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/interface/meshblock_data.hpp b/src/interface/meshblock_data.hpp index 206f9beaf75a..78e7944cecbc 100644 --- a/src/interface/meshblock_data.hpp +++ b/src/interface/meshblock_data.hpp @@ -66,13 +66,6 @@ class MeshBlockData { MeshBlockData() = default; explicit MeshBlockData(const std::string &name) : stage_name_(name) {} - // Constructors for getting sub-containers - // the variables returned are all shallow copies of the src container. - MeshBlockData(const MeshBlockData &src, const std::vector &names, - const std::vector &sparse_ids = {}); - MeshBlockData(const MeshBlockData &src, const std::vector &flags, - const std::vector &sparse_ids = {}); - std::shared_ptr GetBlockSharedPointer() const { if (pmy_block.expired()) { PARTHENON_THROW("Invalid pointer to MeshBlock!"); From 0613951b87552481376940c9b0b6ead4b5c97795 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 20:12:02 -0600 Subject: [PATCH 02/14] beating around the bushes... --- src/interface/data_collection.cpp | 2 +- src/interface/data_collection.hpp | 3 +- src/interface/mesh_data.cpp | 6 +-- src/interface/mesh_data.hpp | 8 ++-- src/interface/meshblock_data.cpp | 37 ++------------ src/interface/meshblock_data.hpp | 80 +++++++++++++++++++++++-------- src/mesh/mesh-amr_loadbalance.cpp | 2 +- src/mesh/mesh.cpp | 2 +- src/mesh/meshblock.cpp | 2 +- src/outputs/history.cpp | 4 +- tst/unit/test_index_split.cpp | 2 +- tst/unit/test_mesh_data.cpp | 2 +- tst/unit/test_sparse_pack.cpp | 4 +- 13 files changed, 82 insertions(+), 72 deletions(-) diff --git a/src/interface/data_collection.cpp b/src/interface/data_collection.cpp index 81e6886e29e4..1df3839d57aa 100644 --- a/src/interface/data_collection.cpp +++ b/src/interface/data_collection.cpp @@ -50,7 +50,7 @@ GetOrAdd_impl(Mesh *pmy_mesh_, std::string md_label = mbd_label + "_part-" + std::to_string(i); if (gmg_level) md_label = md_label + "_gmg-" + std::to_string(*gmg_level); containers_[md_label] = std::make_shared>(mbd_label); - containers_[md_label]->Set(partitions[i], pmy_mesh_); + containers_[md_label]->Initialize(partitions[i], pmy_mesh_); if (gmg_level) { containers_[md_label]->grid = GridIdentifier::two_level_composite(*gmg_level); } else { diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index 913fc8439015..5171690f187c 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -19,6 +19,7 @@ #include #include +#include "basic_types.hpp" #include "utils/error_checking.hpp" namespace parthenon { @@ -56,7 +57,7 @@ class DataCollection { } auto c = std::make_shared(name); - c->Initialize(src.get(), fields, shallow); + c->Initialize(src, fields, shallow); Set(name, c); diff --git a/src/interface/mesh_data.cpp b/src/interface/mesh_data.cpp index e463c58219a0..9fff0a97ec21 100644 --- a/src/interface/mesh_data.cpp +++ b/src/interface/mesh_data.cpp @@ -17,7 +17,7 @@ namespace parthenon { template -void MeshData::Set(BlockList_t blocks, Mesh *pmesh, int ndim) { +void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, int ndim) { const int nblocks = blocks.size(); ndim_ = ndim; block_data_.resize(nblocks); @@ -28,12 +28,12 @@ void MeshData::Set(BlockList_t blocks, Mesh *pmesh, int ndim) { } template -void MeshData::Set(BlockList_t blocks, Mesh *pmesh) { +void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh) { int ndim; if (pmesh != nullptr) { ndim = pmesh->ndim; } - Set(blocks, pmesh, ndim); + Initialize(blocks, pmesh, ndim); } template diff --git a/src/interface/mesh_data.hpp b/src/interface/mesh_data.hpp index cb1a24c187d5..44b3d9d259ac 100644 --- a/src/interface/mesh_data.hpp +++ b/src/interface/mesh_data.hpp @@ -190,6 +190,7 @@ const MeshBlockPack

&PackOnMesh(M &map, BlockDataList_t &block_data_, template class MeshData { public: + using parent_t = Mesh; MeshData() = default; explicit MeshData(const std::string &name) : stage_name_(name) {} @@ -241,11 +242,12 @@ class MeshData { } } - void Set(BlockList_t blocks, Mesh *pmesh, int ndim); - void Set(BlockList_t blocks, Mesh *pmesh); + void Initialize(BlockList_t blocks, Mesh *pmesh, int ndim); + void Initialize(BlockList_t blocks, Mesh *pmesh); template - void Initialize(const MeshData *src, const std::vector &vars, + void Initialize(std::shared_ptr> src, + const std::vector &vars, const bool shallow) { if (src == nullptr) { PARTHENON_THROW("src points at null"); diff --git a/src/interface/meshblock_data.cpp b/src/interface/meshblock_data.cpp index 3017d5c419d4..9a927062f8aa 100644 --- a/src/interface/meshblock_data.cpp +++ b/src/interface/meshblock_data.cpp @@ -33,37 +33,6 @@ #include "utils/utils.hpp" namespace parthenon { - -template -void MeshBlockData::Initialize( - const std::shared_ptr resolved_packages, - const std::shared_ptr pmb) { - SetBlockPointer(pmb); - resolved_packages_ = resolved_packages; - - // clear all variables, maps, and pack caches - varVector_.clear(); - varMap_.clear(); - varUidMap_.clear(); - flagsToVars_.clear(); - varPackMap_.clear(); - coarseVarPackMap_.clear(); - varFluxPackMap_.clear(); - - for (auto const &q : resolved_packages->AllFields()) { - AddField(q.first.base_name, q.second, q.first.sparse_id); - } - - const auto &swarm_container = GetSwarmData(); - swarm_container->Initialize(resolved_packages, pmb); - - Metadata::FlagCollection flags({Metadata::Sparse, Metadata::ForceAllocOnNewBlocks}); - auto vars = GetVariablesByFlag(flags); - for (auto &v : vars.vars()) { - AllocateSparse(v->label()); - } -} - /// /// The internal routine for adding a new field. This subroutine /// is topology aware and will allocate accordingly. @@ -270,9 +239,9 @@ MeshBlockData::GetVariablesByName(const std::vector &names, } else { var_list.Add(v, sparse_ids_set); } - } else if ((resolved_packages_ != nullptr) && - (resolved_packages_->SparseBaseNamePresent(name))) { - const auto &sparse_pool = resolved_packages_->GetSparsePool(name); + } else if ((resolved_packages != nullptr) && + (resolved_packages->SparseBaseNamePresent(name))) { + const auto &sparse_pool = resolved_packages->GetSparsePool(name); // add all sparse ids of the pool for (const auto iter : sparse_pool.pool()) { diff --git a/src/interface/meshblock_data.hpp b/src/interface/meshblock_data.hpp index 78e7944cecbc..60efe07db8c0 100644 --- a/src/interface/meshblock_data.hpp +++ b/src/interface/meshblock_data.hpp @@ -36,6 +36,8 @@ namespace parthenon { +template constexpr std::false_type always_false{}; + /// Interface to underlying infrastructure for data declaration and access. /// /// The MeshBlockData class is a container for the variables that make up @@ -115,18 +117,29 @@ class MeshBlockData { pmy_block = other->GetBlockSharedPointer(); } - void Initialize(const std::shared_ptr resolved_packages, - const std::shared_ptr pmb); - /// Create copy of MeshBlockData, possibly with a subset of named fields, /// and possibly shallow. Note when shallow=false, new storage is allocated /// for non-OneCopy vars, but the data from src is not actually deep copied - template - void Initialize(const MeshBlockData *src, const std::vector &vars, - const bool shallow_copy) { + template + void Initialize(const std::shared_ptr src, + const std::vector &vars = {}, + const bool shallow_copy = false) { + Initialize(src->resolved_packages, src, vars, shallow_copy); + } + + template + void Initialize(const std::shared_ptr resolved_packages_in, + const std::shared_ptr src, + const std::vector &vars = {}, + const bool shallow_copy = false) { + if constexpr (!(std::is_same_v> + || std::is_same_v)) { + // We don't allow other types + static_assert(always_false); + } PARTHENON_DEBUG_REQUIRE(src != nullptr, "Source data must be non-null."); SetBlockPointer(src); - resolved_packages_ = src->resolved_packages_; + resolved_packages = resolved_packages_in; is_shallow_ = shallow_copy; // clear all variables, maps, and pack caches @@ -148,23 +161,48 @@ class MeshBlockData { // special case when the list of vars is empty, copy everything if (vars.empty()) { - for (auto v : src->GetVariableVector()) { - add_var(v); + if constexpr (std::is_same_v>) { + for (auto v : src->GetVariableVector()) { + add_var(v); + } + } else if constexpr (std::is_same_v) { + for (auto const &q : resolved_packages->AllFields()) { + AddField(q.first.base_name, q.second, q.first.sparse_id); + } } } else { - for (const auto &v : vars) { - auto var = src->GetVarPtr(v); - add_var(var); - // Add the associated flux as well if not explicitly - // asked for - if (var->IsSet(Metadata::WithFluxes)) { - auto flx_name = var->metadata().GetFluxName(); - bool found = false; - for (const auto &v2 : vars) { - if (src->GetVarPtr(v2)->label() == flx_name) found = true; + if constexpr (std::is_same_v>) { + for (const auto &v : vars) { + auto var = src->GetVarPtr(v); + add_var(var); + // Add the associated flux as well if not explicitly + // asked for + if (var->IsSet(Metadata::WithFluxes)) { + auto flx_name = var->metadata().GetFluxName(); + bool found = false; + for (const auto &v2 : vars) { + if (src->GetVarPtr(v2)->label() == flx_name) found = true; + } + if (!found) add_var(src->GetVarPtr(flx_name)); } - if (!found) add_var(src->GetVarPtr(flx_name)); } + } else { + PARTHENON_FAIL("Variable subset selection not yet implemented for MeshBlock input."); + } + } + + // TODO(LFR): Not sure why we only do this in the MeshBlock case, but this carries + // over from the previous iteration. + if constexpr (std::is_same_v) { + const auto &swarm_container = GetSwarmData(); + swarm_container->Initialize(resolved_packages, GetBlockSharedPointer()); + + // This seems to work fine outside the constexpr if, but having it inside is consistent + // with the old code. + Metadata::FlagCollection flags({Metadata::Sparse, Metadata::ForceAllocOnNewBlocks}); + auto alloc_vars = GetVariablesByFlag(flags); + for (auto &v : alloc_vars.vars()) { + AllocateSparse(v->label()); } } } @@ -566,7 +604,7 @@ class MeshBlockData { } std::weak_ptr pmy_block; - std::shared_ptr resolved_packages_; + std::shared_ptr resolved_packages; bool is_shallow_ = false; const std::string stage_name_; diff --git a/src/mesh/mesh-amr_loadbalance.cpp b/src/mesh/mesh-amr_loadbalance.cpp index be5e7ec08d0e..7c7b418e4891 100644 --- a/src/mesh/mesh-amr_loadbalance.cpp +++ b/src/mesh/mesh-amr_loadbalance.cpp @@ -993,7 +993,7 @@ void Mesh::RedistributeAndRefineMeshBlocks(ParameterInput *pin, ApplicationInput FillDerived(); // Initialize the "base" MeshData object - mesh_data.Get()->Set(block_list, this); + mesh_data.Get()->Initialize(block_list, this); } // AMR Recv and unpack data ResetLoadBalanceVariables(); diff --git a/src/mesh/mesh.cpp b/src/mesh/mesh.cpp index 000879a589dc..76aa6f9aec9c 100644 --- a/src/mesh/mesh.cpp +++ b/src/mesh/mesh.cpp @@ -1214,7 +1214,7 @@ void Mesh::Initialize(bool init_problem, ParameterInput *pin, ApplicationInput * } while (!init_done); // Initialize the "base" MeshData object - mesh_data.Get()->Set(block_list, this); + mesh_data.Get()->Initialize(block_list, this); } /// Finds location of a block with ID `tgid`. diff --git a/src/mesh/meshblock.cpp b/src/mesh/meshblock.cpp index dff5a86af08c..b36fb6489e91 100644 --- a/src/mesh/meshblock.cpp +++ b/src/mesh/meshblock.cpp @@ -142,7 +142,7 @@ void MeshBlock::Initialize(int igid, int ilid, LogicalLocation iloc, // Resolve issues. auto &real_container = meshblock_data.Get(); - real_container->Initialize(resolved_packages, shared_from_this()); + real_container->Initialize(shared_from_this()); // Initialize swarm boundary condition flags real_container->GetSwarmData()->InitializeBoundaries(shared_from_this()); diff --git a/src/outputs/history.cpp b/src/outputs/history.cpp index 2bd1f1b4fc3f..90188278d7fb 100644 --- a/src/outputs/history.cpp +++ b/src/outputs/history.cpp @@ -83,13 +83,13 @@ void HistoryOutput::WriteOutputFile(Mesh *pm, ParameterInput *pin, SimTime *tm, auto &md_base = pm->mesh_data.Get(); // Populated with all blocks if (md_base->NumBlocks() == 0) { - md_base->Set(pm->block_list, pm); + md_base->Initialize(pm->block_list, pm); } else if (md_base->NumBlocks() != pm->block_list.size()) { PARTHENON_WARN( "Resetting \"base\" MeshData to contain all blocks. This indicates that " "the \"base\" MeshData container has been modified elsewhere. Double check " "that the modification was intentional and is compatible with this reset.") - md_base->Set(pm->block_list, pm); + md_base->Initialize(pm->block_list, pm); } // Loop over all packages of the application diff --git a/tst/unit/test_index_split.cpp b/tst/unit/test_index_split.cpp index a327dc8adbb9..c060f19df325 100644 --- a/tst/unit/test_index_split.cpp +++ b/tst/unit/test_index_split.cpp @@ -99,7 +99,7 @@ TEST_CASE("IndexSplit", "[IndexSplit]") { BlockList_t block_list = MakeBlockList(pkg, NBLOCKS, N, NDIM); MeshData mesh_data("base"); - mesh_data.Set(block_list, nullptr, NDIM); + mesh_data.Initialize(block_list, nullptr, NDIM); WHEN("We initialize an IndexSplit with all outer k and no outer j") { IndexSplit sp(&mesh_data, IndexDomain::interior, IndexSplit::all_outer, diff --git a/tst/unit/test_mesh_data.cpp b/tst/unit/test_mesh_data.cpp index a4cba8f30f8a..f9fa78103ff0 100644 --- a/tst/unit/test_mesh_data.cpp +++ b/tst/unit/test_mesh_data.cpp @@ -78,7 +78,7 @@ TEST_CASE("MeshData works as expected for simple packs", "[MeshData]") { BlockList_t block_list = MakeBlockList(pkg, NBLOCKS, N, NDIM); MeshData mesh_data("base"); - mesh_data.Set(block_list, nullptr); + mesh_data.Initialize(block_list, nullptr); THEN("The number of blocks is correct") { REQUIRE(mesh_data.NumBlocks() == NBLOCKS); } diff --git a/tst/unit/test_sparse_pack.cpp b/tst/unit/test_sparse_pack.cpp index 106dc97befc2..33ffd70e9bb5 100644 --- a/tst/unit/test_sparse_pack.cpp +++ b/tst/unit/test_sparse_pack.cpp @@ -100,7 +100,7 @@ TEST_CASE("Test behavior of sparse packs", "[SparsePack]") { BlockList_t block_list = MakeBlockList(pkg, NBLOCKS, N, NDIM); MeshData mesh_data("base"); - mesh_data.Set(block_list, nullptr); + mesh_data.Initialize(block_list, nullptr); WHEN("We initialize the independent variables by hand and deallocate one") { auto ib = block_list[0]->cellbounds.GetBoundsI(IndexDomain::entire); @@ -168,7 +168,7 @@ TEST_CASE("Test behavior of sparse packs", "[SparsePack]") { BlockList_t block_list = MakeBlockList(pkg, NBLOCKS, N, NDIM); MeshData mesh_data("base"); - mesh_data.Set(block_list, nullptr); + mesh_data.Initialize(block_list, nullptr); WHEN("We initialize the independent variables by hand and deallocate one") { auto ib = block_list[0]->cellbounds.GetBoundsI(IndexDomain::entire); From 2880bd07aba71c68fb78d9f9e3e49aabae2afccc Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 21:02:46 -0600 Subject: [PATCH 03/14] Actually enforce that we only get this on the base stage --- src/interface/meshblock_data.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/interface/meshblock_data.hpp b/src/interface/meshblock_data.hpp index 60efe07db8c0..005711aa55a7 100644 --- a/src/interface/meshblock_data.hpp +++ b/src/interface/meshblock_data.hpp @@ -194,9 +194,11 @@ class MeshBlockData { // TODO(LFR): Not sure why we only do this in the MeshBlock case, but this carries // over from the previous iteration. if constexpr (std::is_same_v) { - const auto &swarm_container = GetSwarmData(); - swarm_container->Initialize(resolved_packages, GetBlockSharedPointer()); - + if (stage_name_ == "base") { + const auto &swarm_container = GetSwarmData(); + swarm_container->Initialize(resolved_packages, GetBlockSharedPointer()); + } + // This seems to work fine outside the constexpr if, but having it inside is consistent // with the old code. Metadata::FlagCollection flags({Metadata::Sparse, Metadata::ForceAllocOnNewBlocks}); From bbd956f181aeeea794bf654e1b92ad13d91d7d34 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 21:03:36 -0600 Subject: [PATCH 04/14] move setting grid to MeshData constructor --- src/interface/data_collection.cpp | 7 +------ src/interface/mesh_data.cpp | 13 +++++++++---- src/interface/mesh_data.hpp | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/interface/data_collection.cpp b/src/interface/data_collection.cpp index 1df3839d57aa..f81a00aa03ed 100644 --- a/src/interface/data_collection.cpp +++ b/src/interface/data_collection.cpp @@ -50,12 +50,7 @@ GetOrAdd_impl(Mesh *pmy_mesh_, std::string md_label = mbd_label + "_part-" + std::to_string(i); if (gmg_level) md_label = md_label + "_gmg-" + std::to_string(*gmg_level); containers_[md_label] = std::make_shared>(mbd_label); - containers_[md_label]->Initialize(partitions[i], pmy_mesh_); - if (gmg_level) { - containers_[md_label]->grid = GridIdentifier::two_level_composite(*gmg_level); - } else { - containers_[md_label]->grid = GridIdentifier::leaf(); - } + containers_[md_label]->Initialize(partitions[i], pmy_mesh_, gmg_level); } } return containers_[label]; diff --git a/src/interface/mesh_data.cpp b/src/interface/mesh_data.cpp index 9fff0a97ec21..0ea6e3da1af5 100644 --- a/src/interface/mesh_data.cpp +++ b/src/interface/mesh_data.cpp @@ -17,23 +17,28 @@ namespace parthenon { template -void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, int ndim) { +void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, std::optional gmg_level) { const int nblocks = blocks.size(); ndim_ = ndim; block_data_.resize(nblocks); SetMeshPointer(pmesh); for (int i = 0; i < nblocks; i++) { - block_data_[i] = blocks[i]->meshblock_data.Get(stage_name_); + block_data_[i] = blocks[i]->meshblock_data.Add(stage_name_, blocks[i]); + } + if (gmg_level) { + grid = GridIdentifier::two_level_composite(*gmg_level); + } else { + grid = GridIdentifier::leaf(); } } template -void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh) { +void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, std::optional gmg_level) { int ndim; if (pmesh != nullptr) { ndim = pmesh->ndim; } - Initialize(blocks, pmesh, ndim); + Initialize(blocks, pmesh, ndim, gmg_level); } template diff --git a/src/interface/mesh_data.hpp b/src/interface/mesh_data.hpp index 44b3d9d259ac..1b59d9887ef0 100644 --- a/src/interface/mesh_data.hpp +++ b/src/interface/mesh_data.hpp @@ -242,8 +242,8 @@ class MeshData { } } - void Initialize(BlockList_t blocks, Mesh *pmesh, int ndim); - void Initialize(BlockList_t blocks, Mesh *pmesh); + void Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, std::optional gmg_level = {}); + void Initialize(BlockList_t blocks, Mesh *pmesh, std::optional gmg_level = {}); template void Initialize(std::shared_ptr> src, From dd6498e023d7a4c670129af2011af3b6b4377749 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 21:04:28 -0600 Subject: [PATCH 05/14] Generalize Add to take a different source --- src/interface/data_collection.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index 5171690f187c..e52407a74797 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -45,8 +45,8 @@ class DataCollection { void SetMeshPointer(Mesh *pmesh) { pmy_mesh_ = pmesh; } - template - std::shared_ptr &Add(const std::string &name, const std::shared_ptr &src, + template + std::shared_ptr &Add(const std::string &name, const std::shared_ptr &src, const std::vector &fields, const bool shallow) { auto it = containers_.find(name); if (it != containers_.end()) { @@ -63,13 +63,13 @@ class DataCollection { return containers_[name]; } - template - std::shared_ptr &Add(const std::string &label, const std::shared_ptr &src, + template + std::shared_ptr &Add(const std::string &label, const std::shared_ptr &src, const std::vector &fields = {}) { return Add(label, src, fields, false); } - template - std::shared_ptr &AddShallow(const std::string &label, const std::shared_ptr &src, + template + std::shared_ptr &AddShallow(const std::string &label, const std::shared_ptr &src, const std::vector &fields = {}) { return Add(label, src, fields, true); } From 3a33ff6ab73ce0fe518c939044e27ad0151a026e Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 21:04:52 -0600 Subject: [PATCH 06/14] Remove boiler plate --- example/fine_advection/advection_driver.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/example/fine_advection/advection_driver.cpp b/example/fine_advection/advection_driver.cpp index 486279cfaada..f0a7df734a0f 100644 --- a/example/fine_advection/advection_driver.cpp +++ b/example/fine_advection/advection_driver.cpp @@ -62,17 +62,6 @@ TaskCollection AdvectionDriver::MakeTaskCollection(BlockList_t &blocks, const in // Build MeshBlockData containers that will be included in MeshData containers. It is // gross that this has to be done by hand. const auto &stage_name = integrator->stage_name; - if (stage == 1) { - for (int i = 0; i < blocks.size(); i++) { - auto &pmb = blocks[i]; - // first make other useful containers - auto &base = pmb->meshblock_data.Get(); - pmb->meshblock_data.Add("dUdt", base); - for (int s = 1; s < integrator->nstages; s++) - pmb->meshblock_data.Add(stage_name[s], base); - } - } - const Real beta = integrator->beta[stage - 1]; const Real dt = integrator->dt; From 8982aa637d72af22e0b0068cad81c58adcb1225c Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 21:16:40 -0600 Subject: [PATCH 07/14] Move always_false to concepts_lite --- src/interface/meshblock_data.hpp | 4 +--- src/utils/concepts_lite.hpp | 6 ++++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/interface/meshblock_data.hpp b/src/interface/meshblock_data.hpp index 005711aa55a7..f3931d624a9f 100644 --- a/src/interface/meshblock_data.hpp +++ b/src/interface/meshblock_data.hpp @@ -31,13 +31,11 @@ #include "interface/variable.hpp" #include "interface/variable_pack.hpp" #include "mesh/domain.hpp" +#include "utils/concepts_lite.hpp" #include "utils/error_checking.hpp" #include "utils/unique_id.hpp" namespace parthenon { - -template constexpr std::false_type always_false{}; - /// Interface to underlying infrastructure for data declaration and access. /// /// The MeshBlockData class is a container for the variables that make up diff --git a/src/utils/concepts_lite.hpp b/src/utils/concepts_lite.hpp index 43d2344cd855..7da0ccb87131 100644 --- a/src/utils/concepts_lite.hpp +++ b/src/utils/concepts_lite.hpp @@ -16,6 +16,12 @@ #include #include +// This is a class template that is required for doing something like static_assert(false) +// in constexpr if blocks. Actually writing static_assert(false) will always cause a +// compilation error, even if it is an unchosen constexpr if block. This is fixed in C++23 +// I think. +template constexpr std::false_type always_false{}; + // These macros are just to make code more readable and self-explanatory, // generally it is best to write template<..., REQUIRES(... && ...)> in the code // but there are some instance where this causes issues. Switching to the construct From 3729dae3bb522e7dffe17947f5b760cceae2e6ff Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 21:17:05 -0600 Subject: [PATCH 08/14] Add type checking comment --- src/interface/data_collection.hpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index e52407a74797..9ac5d4f4c00c 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -20,6 +20,7 @@ #include #include "basic_types.hpp" +#include "utils/concepts_lite.hpp" #include "utils/error_checking.hpp" namespace parthenon { @@ -48,6 +49,14 @@ class DataCollection { template std::shared_ptr &Add(const std::string &name, const std::shared_ptr &src, const std::vector &fields, const bool shallow) { + // This is commented out for now, as we don't include headers for the types below + //if constexpr (!( + // (std::is_same_v && std::is_same_v>) + // || std::is_same_v)) { + // // SRC_t and T are incompatible + // static_assert(alway_false); + //} + auto it = containers_.find(name); if (it != containers_.end()) { if (fields.size() && !(it->second)->Contains(fields)) { From 33f6cede5c014b4cc99525f5d81dadda7b0108c8 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 22:18:00 -0600 Subject: [PATCH 09/14] cleanup and make Add work for MeshData --- src/interface/data_collection.cpp | 20 +++++++------- src/interface/data_collection.hpp | 45 +++++++++++++++++++++++-------- src/interface/mesh_data.hpp | 1 + 3 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/interface/data_collection.cpp b/src/interface/data_collection.cpp index f81a00aa03ed..d537e9ba22e0 100644 --- a/src/interface/data_collection.cpp +++ b/src/interface/data_collection.cpp @@ -32,25 +32,25 @@ std::shared_ptr &DataCollection::Add(const std::string &label) { return containers_[label]; } +template <> std::shared_ptr> & -GetOrAdd_impl(Mesh *pmy_mesh_, - std::map>> &containers_, - BlockList_t &block_list, const std::string &mbd_label, +DataCollection>::GetOrAdd_impl( + const std::string &mbd_label, const int &partition_id, const std::optional gmg_level) { - std::string label = mbd_label + "_part-" + std::to_string(partition_id); - if (gmg_level) label = label + "_gmg-" + std::to_string(*gmg_level); + std::string label = GetKey(mbd_label, partition_id, gmg_level); auto it = containers_.find(label); if (it == containers_.end()) { // TODO(someone) add caching of partitions to Mesh at some point const int pack_size = pmy_mesh_->DefaultPackSize(); + auto &block_list = gmg_level ? pmy_mesh_->gmg_block_lists[*gmg_level] : pmy_mesh_->block_list; auto partitions = partition::ToSizeN(block_list, pack_size); // Account for possibly empty block_list if (partitions.size() == 0) partitions = std::vector(1); for (auto i = 0; i < partitions.size(); i++) { - std::string md_label = mbd_label + "_part-" + std::to_string(i); - if (gmg_level) md_label = md_label + "_gmg-" + std::to_string(*gmg_level); + std::string md_label = GetKey(mbd_label, partition_id, gmg_level); containers_[md_label] = std::make_shared>(mbd_label); containers_[md_label]->Initialize(partitions[i], pmy_mesh_, gmg_level); + containers_[md_label]->partition = i; } } return containers_[label]; @@ -60,16 +60,14 @@ template <> std::shared_ptr> & DataCollection>::GetOrAdd(const std::string &mbd_label, const int &partition_id) { - return GetOrAdd_impl(pmy_mesh_, containers_, pmy_mesh_->block_list, mbd_label, - partition_id, {}); + return GetOrAdd_impl(mbd_label, partition_id, {}); } template <> std::shared_ptr> & DataCollection>::GetOrAdd(int gmg_level, const std::string &mbd_label, const int &partition_id) { - return GetOrAdd_impl(pmy_mesh_, containers_, pmy_mesh_->gmg_block_lists[gmg_level], - mbd_label, partition_id, gmg_level); + return GetOrAdd_impl(mbd_label, partition_id, gmg_level); } template class DataCollection>; diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index 9ac5d4f4c00c..a0782b35856e 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -25,6 +25,11 @@ namespace parthenon { class Mesh; +class MeshBlock; +template +class MeshData; +template +class MeshBlockData; /// The DataCollection class is an abstract container that contains at least a /// "base" container of some type (e.g., of MeshData or MeshBlockData) plus /// additional containers identified by string labels. @@ -49,13 +54,12 @@ class DataCollection { template std::shared_ptr &Add(const std::string &name, const std::shared_ptr &src, const std::vector &fields, const bool shallow) { - // This is commented out for now, as we don't include headers for the types below - //if constexpr (!( - // (std::is_same_v && std::is_same_v>) - // || std::is_same_v)) { - // // SRC_t and T are incompatible - // static_assert(alway_false); - //} + if constexpr (!( + (std::is_same_v && std::is_same_v>) + || std::is_same_v)) { + // SRC_t and T are incompatible + static_assert(always_false); + } auto it = containers_.find(name); if (it != containers_.end()) { @@ -67,10 +71,9 @@ class DataCollection { auto c = std::make_shared(name); c->Initialize(src, fields, shallow); - - Set(name, c); - - return containers_[name]; + auto key = GetKey(name, src); + containers_[key] = c; + return containers_[key]; } template std::shared_ptr &Add(const std::string &label, const std::shared_ptr &src, @@ -115,6 +118,26 @@ class DataCollection { } private: + template + std::string GetKey(const std::string& stage_label, const std::shared_ptr&in) { + if constexpr (std::is_same_v>) { + std::string key = stage_label + "_part-" + std::to_string(in->partition); + if (in->grid.type == GridType::two_level_composite) key = key + "_gmg-" + std::to_string(in->grid.logical_level); + return key; + } else { + return stage_label; + } + } + std::string GetKey(const std::string& stage_label, std::optional partition_id, std::optional gmg_level) { + std::string key = stage_label; + if (partition_id) key = key + "_part-" + std::to_string(*partition_id); + if (gmg_level) key = key + "_gmg-" + std::to_string(*gmg_level); + return key; + } + + std::shared_ptr &GetOrAdd_impl(const std::string &mbd_label, + const int &partition_id, const std::optional gmg_level); + Mesh *pmy_mesh_; std::map> containers_; }; diff --git a/src/interface/mesh_data.hpp b/src/interface/mesh_data.hpp index 1b59d9887ef0..7015e441d006 100644 --- a/src/interface/mesh_data.hpp +++ b/src/interface/mesh_data.hpp @@ -195,6 +195,7 @@ class MeshData { explicit MeshData(const std::string &name) : stage_name_(name) {} GridIdentifier grid; + int partition; const auto &StageName() const { return stage_name_; } From b4facf0d11761f1a9d2f7b491e72b5c6306cd44c Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Tue, 28 May 2024 22:19:23 -0600 Subject: [PATCH 10/14] format --- src/interface/data_collection.cpp | 9 +++++---- src/interface/data_collection.hpp | 31 +++++++++++++++++-------------- src/interface/mesh_data.cpp | 6 ++++-- src/interface/mesh_data.hpp | 6 +++--- src/interface/meshblock_data.hpp | 23 +++++++++++------------ src/utils/concepts_lite.hpp | 7 ++++--- 6 files changed, 44 insertions(+), 38 deletions(-) diff --git a/src/interface/data_collection.cpp b/src/interface/data_collection.cpp index d537e9ba22e0..20ad1b9a2836 100644 --- a/src/interface/data_collection.cpp +++ b/src/interface/data_collection.cpp @@ -34,15 +34,16 @@ std::shared_ptr &DataCollection::Add(const std::string &label) { template <> std::shared_ptr> & -DataCollection>::GetOrAdd_impl( - const std::string &mbd_label, - const int &partition_id, const std::optional gmg_level) { +DataCollection>::GetOrAdd_impl(const std::string &mbd_label, + const int &partition_id, + const std::optional gmg_level) { std::string label = GetKey(mbd_label, partition_id, gmg_level); auto it = containers_.find(label); if (it == containers_.end()) { // TODO(someone) add caching of partitions to Mesh at some point const int pack_size = pmy_mesh_->DefaultPackSize(); - auto &block_list = gmg_level ? pmy_mesh_->gmg_block_lists[*gmg_level] : pmy_mesh_->block_list; + auto &block_list = + gmg_level ? pmy_mesh_->gmg_block_lists[*gmg_level] : pmy_mesh_->block_list; auto partitions = partition::ToSizeN(block_list, pack_size); // Account for possibly empty block_list if (partitions.size() == 0) partitions = std::vector(1); diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index a0782b35856e..9c34ae7086a1 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -26,9 +26,9 @@ namespace parthenon { class Mesh; class MeshBlock; -template +template class MeshData; -template +template class MeshBlockData; /// The DataCollection class is an abstract container that contains at least a /// "base" container of some type (e.g., of MeshData or MeshBlockData) plus @@ -54,9 +54,9 @@ class DataCollection { template std::shared_ptr &Add(const std::string &name, const std::shared_ptr &src, const std::vector &fields, const bool shallow) { - if constexpr (!( - (std::is_same_v && std::is_same_v>) - || std::is_same_v)) { + if constexpr (!((std::is_same_v && + std::is_same_v>) || + std::is_same_v)) { // SRC_t and T are incompatible static_assert(always_false); } @@ -81,7 +81,8 @@ class DataCollection { return Add(label, src, fields, false); } template - std::shared_ptr &AddShallow(const std::string &label, const std::shared_ptr &src, + std::shared_ptr &AddShallow(const std::string &label, + const std::shared_ptr &src, const std::vector &fields = {}) { return Add(label, src, fields, true); } @@ -119,24 +120,26 @@ class DataCollection { private: template - std::string GetKey(const std::string& stage_label, const std::shared_ptr&in) { - if constexpr (std::is_same_v>) { + std::string GetKey(const std::string &stage_label, const std::shared_ptr &in) { + if constexpr (std::is_same_v>) { std::string key = stage_label + "_part-" + std::to_string(in->partition); - if (in->grid.type == GridType::two_level_composite) key = key + "_gmg-" + std::to_string(in->grid.logical_level); + if (in->grid.type == GridType::two_level_composite) + key = key + "_gmg-" + std::to_string(in->grid.logical_level); return key; - } else { + } else { return stage_label; } } - std::string GetKey(const std::string& stage_label, std::optional partition_id, std::optional gmg_level) { + std::string GetKey(const std::string &stage_label, std::optional partition_id, + std::optional gmg_level) { std::string key = stage_label; if (partition_id) key = key + "_part-" + std::to_string(*partition_id); - if (gmg_level) key = key + "_gmg-" + std::to_string(*gmg_level); + if (gmg_level) key = key + "_gmg-" + std::to_string(*gmg_level); return key; } - std::shared_ptr &GetOrAdd_impl(const std::string &mbd_label, - const int &partition_id, const std::optional gmg_level); + std::shared_ptr &GetOrAdd_impl(const std::string &mbd_label, const int &partition_id, + const std::optional gmg_level); Mesh *pmy_mesh_; std::map> containers_; diff --git a/src/interface/mesh_data.cpp b/src/interface/mesh_data.cpp index 0ea6e3da1af5..bc1a2e91556a 100644 --- a/src/interface/mesh_data.cpp +++ b/src/interface/mesh_data.cpp @@ -17,7 +17,8 @@ namespace parthenon { template -void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, std::optional gmg_level) { +void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, + std::optional gmg_level) { const int nblocks = blocks.size(); ndim_ = ndim; block_data_.resize(nblocks); @@ -33,7 +34,8 @@ void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, std::opt } template -void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, std::optional gmg_level) { +void MeshData::Initialize(BlockList_t blocks, Mesh *pmesh, + std::optional gmg_level) { int ndim; if (pmesh != nullptr) { ndim = pmesh->ndim; diff --git a/src/interface/mesh_data.hpp b/src/interface/mesh_data.hpp index 7015e441d006..4bde8c311017 100644 --- a/src/interface/mesh_data.hpp +++ b/src/interface/mesh_data.hpp @@ -243,12 +243,12 @@ class MeshData { } } - void Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, std::optional gmg_level = {}); + void Initialize(BlockList_t blocks, Mesh *pmesh, int ndim, + std::optional gmg_level = {}); void Initialize(BlockList_t blocks, Mesh *pmesh, std::optional gmg_level = {}); template - void Initialize(std::shared_ptr> src, - const std::vector &vars, + void Initialize(std::shared_ptr> src, const std::vector &vars, const bool shallow) { if (src == nullptr) { PARTHENON_THROW("src points at null"); diff --git a/src/interface/meshblock_data.hpp b/src/interface/meshblock_data.hpp index f3931d624a9f..bfc7e4a0baee 100644 --- a/src/interface/meshblock_data.hpp +++ b/src/interface/meshblock_data.hpp @@ -119,19 +119,17 @@ class MeshBlockData { /// and possibly shallow. Note when shallow=false, new storage is allocated /// for non-OneCopy vars, but the data from src is not actually deep copied template - void Initialize(const std::shared_ptr src, - const std::vector &vars = {}, + void Initialize(const std::shared_ptr src, const std::vector &vars = {}, const bool shallow_copy = false) { Initialize(src->resolved_packages, src, vars, shallow_copy); } - + template void Initialize(const std::shared_ptr resolved_packages_in, - const std::shared_ptr src, - const std::vector &vars = {}, + const std::shared_ptr src, const std::vector &vars = {}, const bool shallow_copy = false) { - if constexpr (!(std::is_same_v> - || std::is_same_v)) { + if constexpr (!(std::is_same_v> || + std::is_same_v)) { // We don't allow other types static_assert(always_false); } @@ -185,11 +183,12 @@ class MeshBlockData { } } } else { - PARTHENON_FAIL("Variable subset selection not yet implemented for MeshBlock input."); + PARTHENON_FAIL( + "Variable subset selection not yet implemented for MeshBlock input."); } } - - // TODO(LFR): Not sure why we only do this in the MeshBlock case, but this carries + + // TODO(LFR): Not sure why we only do this in the MeshBlock case, but this carries // over from the previous iteration. if constexpr (std::is_same_v) { if (stage_name_ == "base") { @@ -197,8 +196,8 @@ class MeshBlockData { swarm_container->Initialize(resolved_packages, GetBlockSharedPointer()); } - // This seems to work fine outside the constexpr if, but having it inside is consistent - // with the old code. + // This seems to work fine outside the constexpr if, but having it inside is + // consistent with the old code. Metadata::FlagCollection flags({Metadata::Sparse, Metadata::ForceAllocOnNewBlocks}); auto alloc_vars = GetVariablesByFlag(flags); for (auto &v : alloc_vars.vars()) { diff --git a/src/utils/concepts_lite.hpp b/src/utils/concepts_lite.hpp index 7da0ccb87131..b3b49d756198 100644 --- a/src/utils/concepts_lite.hpp +++ b/src/utils/concepts_lite.hpp @@ -16,11 +16,12 @@ #include #include -// This is a class template that is required for doing something like static_assert(false) -// in constexpr if blocks. Actually writing static_assert(false) will always cause a +// This is a class template that is required for doing something like static_assert(false) +// in constexpr if blocks. Actually writing static_assert(false) will always cause a // compilation error, even if it is an unchosen constexpr if block. This is fixed in C++23 // I think. -template constexpr std::false_type always_false{}; +template +constexpr std::false_type always_false{}; // These macros are just to make code more readable and self-explanatory, // generally it is best to write template<..., REQUIRES(... && ...)> in the code From 5909597df58b4971faa525634193d154520efdfc Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Wed, 29 May 2024 11:17:42 -0600 Subject: [PATCH 11/14] Actually check the key instead of the name --- src/interface/data_collection.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index 9c34ae7086a1..e553cf21d8c2 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -61,17 +61,18 @@ class DataCollection { static_assert(always_false); } - auto it = containers_.find(name); + auto key = GetKey(name, src); + auto it = containers_.find(key); if (it != containers_.end()) { if (fields.size() && !(it->second)->Contains(fields)) { - PARTHENON_THROW(name + " already exists in collection but fields do not match."); + PARTHENON_THROW(key + " already exists in collection but fields do not match."); } return it->second; } auto c = std::make_shared(name); c->Initialize(src, fields, shallow); - auto key = GetKey(name, src); + containers_[key] = c; return containers_[key]; } From 58cc64dc778d60cbfef5a4c1d593c227f305fa34 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Wed, 29 May 2024 12:52:58 -0600 Subject: [PATCH 12/14] format --- src/interface/data_collection.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index e553cf21d8c2..34682fbf10cb 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -72,7 +72,7 @@ class DataCollection { auto c = std::make_shared(name); c->Initialize(src, fields, shallow); - + containers_[key] = c; return containers_[key]; } From 9c53bae4834dfa818d025dbab39a8976fcfb15e8 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Wed, 29 May 2024 13:28:53 -0600 Subject: [PATCH 13/14] Add comments to static_asserts --- src/interface/data_collection.hpp | 2 +- src/interface/meshblock_data.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interface/data_collection.hpp b/src/interface/data_collection.hpp index 34682fbf10cb..29551d35290c 100644 --- a/src/interface/data_collection.hpp +++ b/src/interface/data_collection.hpp @@ -58,7 +58,7 @@ class DataCollection { std::is_same_v>) || std::is_same_v)) { // SRC_t and T are incompatible - static_assert(always_false); + static_assert(always_false, "Incompatible source and container types."); } auto key = GetKey(name, src); diff --git a/src/interface/meshblock_data.hpp b/src/interface/meshblock_data.hpp index bfc7e4a0baee..a993bbb07208 100644 --- a/src/interface/meshblock_data.hpp +++ b/src/interface/meshblock_data.hpp @@ -131,7 +131,7 @@ class MeshBlockData { if constexpr (!(std::is_same_v> || std::is_same_v)) { // We don't allow other types - static_assert(always_false); + static_assert(always_false, "Bad source type for initialization."); } PARTHENON_DEBUG_REQUIRE(src != nullptr, "Source data must be non-null."); SetBlockPointer(src); From 0fc1d55cb53ffb3abf45d0d1e06009b5c90ca462 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Wed, 29 May 2024 13:43:48 -0600 Subject: [PATCH 14/14] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a6c145f9836..1f6de004084c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - [[PR 1004]](https://github.com/parthenon-hpc-lab/parthenon/pull/1004) Allow parameter modification from an input file for restarts ### Fixed (not changing behavior/API/variables/...) +- [[PR 1088]](https://github.com/parthenon-hpc-lab/parthenon/pull/1088) Updates to DataCollection and MeshData to remove requirement of predefining MeshBlockData - [[PR 1088]](https://github.com/parthenon-hpc-lab/parthenon/pull/1088) Correctly fill fluxes for non-cell variables in SparsePacks - [[PR 1083]](https://github.com/parthenon-hpc-lab/parthenon/pull/1083) Correctly fill VariableFluxPack for edge fluxes in 2D - [[PR 1087]](https://github.com/parthenon-hpc-lab/parthenon/pull/1087) Make sure InnerLoopPatternTVR is resolved on device properly when it is the default loop pattern