diff --git a/CHANGELOG.md b/CHANGELOG.md index 79de1831b62e..6e6f305ded88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - [[PR 359]](https://github.com/lanl/parthenon/pull/359) MeshBlockPack support for buffer pack and unpack of CellCentered Variables ### Changed (changing behavior/API/variables/...) +- [[PR 425]](https://github.com/lanl/parthenon/pull/425) Remove ambiguity in package names. `Packages_t` no longer has an `operator[]` method. This has been replaced with `Add` and `Get`. - [[PR 359]](https://github.com/lanl/parthenon/pull/359) Templated inline reconstruction functions to support different types (e.g., `ParArray4D` or `ParArrayND`) ### Fixed (not changing behavior/API/variables/...) diff --git a/example/advection/advection_driver.cpp b/example/advection/advection_driver.cpp index 55f86ac2c5c8..08cd6e17047a 100644 --- a/example/advection/advection_driver.cpp +++ b/example/advection/advection_driver.cpp @@ -122,7 +122,7 @@ TaskCollection AdvectionDriver::MakeTaskCollection(BlockList_t &blocks, const in } const auto &buffer_send_pack = - blocks[0]->packages["advection_package"]->Param("buffer_send_pack"); + blocks[0]->packages.Get("advection_package")->Param("buffer_send_pack"); if (buffer_send_pack) { TaskRegion &tr = tc.AddRegion(num_partitions); for (int i = 0; i < num_partitions; i++) { @@ -138,7 +138,7 @@ TaskCollection AdvectionDriver::MakeTaskCollection(BlockList_t &blocks, const in } const auto &buffer_recv_pack = - blocks[0]->packages["advection_package"]->Param("buffer_recv_pack"); + blocks[0]->packages.Get("advection_package")->Param("buffer_recv_pack"); if (buffer_recv_pack) { TaskRegion &tr = tc.AddRegion(num_partitions); for (int i = 0; i < num_partitions; i++) { @@ -154,7 +154,7 @@ TaskCollection AdvectionDriver::MakeTaskCollection(BlockList_t &blocks, const in } const auto &buffer_set_pack = - blocks[0]->packages["advection_package"]->Param("buffer_set_pack"); + blocks[0]->packages.Get("advection_package")->Param("buffer_set_pack"); if (buffer_set_pack) { TaskRegion &tr = tc.AddRegion(num_partitions); for (int i = 0; i < num_partitions; i++) { diff --git a/example/advection/advection_package.cpp b/example/advection/advection_package.cpp index a878119692f6..50cdd785cecb 100644 --- a/example/advection/advection_package.cpp +++ b/example/advection/advection_package.cpp @@ -198,7 +198,7 @@ AmrTag CheckRefinement(MeshBlockData *rc) { }, Kokkos::MinMax(minmax)); - auto pkg = pmb->packages["advection_package"]; + auto pkg = pmb->packages.Get("advection_package"); const auto &refine_tol = pkg->Param("refine_tol"); const auto &derefine_tol = pkg->Param("derefine_tol"); @@ -279,7 +279,7 @@ void PostFill(MeshBlockData *rc) { // provide the routine that estimates a stable timestep for this package Real EstimateTimestepBlock(MeshBlockData *rc) { auto pmb = rc->GetBlockPointer(); - auto pkg = pmb->packages["advection_package"]; + auto pkg = pmb->packages.Get("advection_package"); const auto &cfl = pkg->Param("cfl"); const auto &vx = pkg->Param("vx"); const auto &vy = pkg->Param("vy"); @@ -319,7 +319,7 @@ TaskStatus CalculateFluxes(std::shared_ptr> &rc) { IndexRange kb = pmb->cellbounds.GetBoundsK(IndexDomain::interior); ParArrayND advected = rc->Get("advected").data; - auto pkg = pmb->packages["advection_package"]; + auto pkg = pmb->packages.Get("advection_package"); const auto &vx = pkg->Param("vx"); const auto &vy = pkg->Param("vy"); const auto &vz = pkg->Param("vz"); diff --git a/example/advection/parthenon_app_inputs.cpp b/example/advection/parthenon_app_inputs.cpp index abbe26b68527..5871865bafb3 100644 --- a/example/advection/parthenon_app_inputs.cpp +++ b/example/advection/parthenon_app_inputs.cpp @@ -35,7 +35,7 @@ void ProblemGenerator(MeshBlock *pmb, ParameterInput *pin) { auto &rc = pmb->meshblock_data.Get(); auto &q = rc->Get("advected").data; - auto pkg = pmb->packages["advection_package"]; + auto pkg = pmb->packages.Get("advection_package"); const auto & = pkg->Param("amp"); const auto &vel = pkg->Param("vel"); const auto &k_par = pkg->Param("k_par"); @@ -93,7 +93,7 @@ void UserWorkAfterLoop(Mesh *mesh, ParameterInput *pin, SimTime &tm) { Real max_err = 0.0; for (auto &pmb : mesh->block_list) { - auto pkg = pmb->packages["advection_package"]; + auto pkg = pmb->packages.Get("advection_package"); auto rc = pmb->meshblock_data.Get(); // get base container const auto & = pkg->Param("amp"); @@ -212,12 +212,12 @@ void UserWorkAfterLoop(Mesh *mesh, ParameterInput *pin, SimTime &tm) { Packages_t ProcessPackages(std::unique_ptr &pin) { Packages_t packages; auto pkg = advection_package::Initialize(pin.get()); - packages[pkg->label()] = pkg; + packages.Add(pkg); auto app = std::make_shared("advection_app"); app->PreFillDerivedBlock = advection_package::PreFill; app->PostFillDerivedBlock = advection_package::PostFill; - packages[app->label()] = app; + packages.Add(app); return packages; } diff --git a/example/calculate_pi/calculate_pi.cpp b/example/calculate_pi/calculate_pi.cpp index bf9a0b699353..31b38c5292ed 100644 --- a/example/calculate_pi/calculate_pi.cpp +++ b/example/calculate_pi/calculate_pi.cpp @@ -41,7 +41,7 @@ void SetInOrOut(MeshBlockData *rc) { IndexRange jb = pmb->cellbounds.GetBoundsJ(IndexDomain::interior); IndexRange kb = pmb->cellbounds.GetBoundsK(IndexDomain::interior); ParArrayND &v = rc->Get("in_or_out").data; - const auto &radius = pmb->packages["calculate_pi"]->Param("radius"); + const auto &radius = pmb->packages.Get("calculate_pi")->Param("radius"); auto &coords = pmb->coords; // Set an indicator function that indicates whether the cell center // is inside or outside of the circle we're interating the area of. @@ -103,7 +103,7 @@ TaskStatus ComputeArea(std::shared_ptr> &md, ParArrayHost a } TaskStatus AccumulateAreas(ParArrayHost areas, Packages_t &packages) { - const auto &radius = packages["calculate_pi"]->Param("radius"); + const auto &radius = packages.Get("calculate_pi")->Param("radius"); Real area = 0.0; for (int i = 0; i < areas.GetSize(); i++) { @@ -119,7 +119,7 @@ TaskStatus AccumulateAreas(ParArrayHost areas, Packages_t &packages) { Real pi_val = area; #endif - packages["calculate_pi"]->AddParam("pi_val", pi_val); + packages.Get("calculate_pi")->AddParam("pi_val", pi_val); return TaskStatus::complete; } diff --git a/example/calculate_pi/pi_driver.cpp b/example/calculate_pi/pi_driver.cpp index b5e9f295d51e..c03d9f4e31d2 100644 --- a/example/calculate_pi/pi_driver.cpp +++ b/example/calculate_pi/pi_driver.cpp @@ -67,7 +67,7 @@ int main(int argc, char *argv[]) { Packages_t ProcessPackages(std::unique_ptr &pin) { Packages_t packages; // only have one package for this app, but will typically have more things added to - packages["calculate_pi"] = calculate_pi::Initialize(pin.get()); + packages.Add(calculate_pi::Initialize(pin.get())); return packages; } @@ -90,7 +90,7 @@ parthenon::DriverStatus PiDriver::Execute() { ConstructAndExecuteTaskLists<>(this); // retrieve "pi_val" and post execute. - auto &pi_val = pmesh->packages["calculate_pi"]->Param("pi_val"); + auto &pi_val = pmesh->packages.Get("calculate_pi")->Param("pi_val"); pmesh->mbcnt = pmesh->nbtotal; // this is how many blocks were processed PostExecute(pi_val); return DriverStatus::complete; diff --git a/example/face_fields/face_fields_example.cpp b/example/face_fields/face_fields_example.cpp index 2697d5a6a55a..d2cbeb508645 100644 --- a/example/face_fields/face_fields_example.cpp +++ b/example/face_fields/face_fields_example.cpp @@ -57,7 +57,7 @@ Packages_t ProcessPackages(std::unique_ptr &pin) { array_size); package->AddField("f.f.face_averaged_value", m); - packages["FaceFieldExample"] = package; + packages.Add(package); return packages; } @@ -167,7 +167,7 @@ TaskList FaceFieldExample::MakeTaskList(MeshBlock *pmb) { parthenon::TaskStatus fill_faces(parthenon::MeshBlock *pmb) { using parthenon::Real; - auto example = pmb->packages["FaceFieldExample"]; + auto example = pmb->packages.Get("FaceFieldExample"); Real px = example->Param("px"); Real py = example->Param("py"); Real pz = example->Param("pz"); diff --git a/example/particles/particles.cpp b/example/particles/particles.cpp index c0bcb7068c5a..549cd36c2bb0 100644 --- a/example/particles/particles.cpp +++ b/example/particles/particles.cpp @@ -27,7 +27,7 @@ namespace particles_example { Packages_t ProcessPackages(std::unique_ptr &pin) { Packages_t packages; - packages["particles_package"] = particles_example::Particles::Initialize(pin.get()); + packages.Add(particles_example::Particles::Initialize(pin.get())); return packages; } @@ -82,7 +82,7 @@ AmrTag CheckRefinement(MeshBlockData *rc) { return AmrTag::same; } Real EstimateTimestepBlock(MeshBlockData *rc) { auto pmb = rc->GetBlockPointer(); - auto pkg = pmb->packages["particles_package"]; + auto pkg = pmb->packages.Get("particles_package"); const Real &dt = pkg->Param("const_dt"); return dt; } @@ -97,7 +97,7 @@ Real EstimateTimestepBlock(MeshBlockData *rc) { // first some helper tasks TaskStatus DestroySomeParticles(MeshBlock *pmb) { - auto pkg = pmb->packages["particles_package"]; + auto pkg = pmb->packages.Get("particles_package"); auto swarm = pmb->swarm_data.Get()->Get("my particles"); auto rng_pool = pkg->Param("rng_pool"); @@ -170,7 +170,7 @@ TaskStatus DepositParticles(MeshBlock *pmb) { } TaskStatus CreateSomeParticles(MeshBlock *pmb) { - auto pkg = pmb->packages["particles_package"]; + auto pkg = pmb->packages.Get("particles_package"); auto swarm = pmb->swarm_data.Get()->Get("my particles"); auto rng_pool = pkg->Param("rng_pool"); auto num_particles = pkg->Param("num_particles"); diff --git a/src/interface/state_descriptor.cpp b/src/interface/state_descriptor.cpp index 0054878eba5e..65e9c9300d0f 100644 --- a/src/interface/state_descriptor.cpp +++ b/src/interface/state_descriptor.cpp @@ -167,13 +167,13 @@ class SparseProvider : public VariableProvider { void AddPrivate(const std::string &package, const std::string &var, const Metadata &metadata) { const std::string name = package + "::" + var; - for (auto &m : packages_[package]->AllSparseFields().at(var)) { + for (auto &m : packages_.Get(package)->AllSparseFields().at(var)) { state_->AddField(name, m.second); } } void AddProvides(const std::string &package, const std::string &var, const Metadata &metadata) { - for (auto &m : packages_[package]->AllSparseFields().at(var)) { + for (auto &m : packages_.Get(package)->AllSparseFields().at(var)) { state_->AddField(var, m.second); } } @@ -192,15 +192,15 @@ class SwarmProvider : public VariableProvider { : packages_(packages), state_(sd) {} void AddPrivate(const std::string &package, const std::string &var, const Metadata &metadata) { - AddSwarm_(packages_[package].get(), var, package + "::" + var, metadata); + AddSwarm_(packages_.Get(package).get(), var, package + "::" + var, metadata); } void AddProvides(const std::string &package, const std::string &var, const Metadata &metadata) { - AddSwarm_(packages_[package].get(), var, var, metadata); + AddSwarm_(packages_.Get(package).get(), var, var, metadata); } void AddOverridable(const std::string &swarm, Metadata &metadata) { state_->AddSwarm(swarm, metadata); - for (auto &pair : packages_) { + for (auto &pair : packages_.AllPackages()) { auto &package = pair.second; if (package->SwarmPresent(swarm)) { for (auto &pair : package->AllSwarmValues(swarm)) { @@ -343,22 +343,21 @@ std::shared_ptr ResolvePackages(Packages_t &packages) { // Add private/provides variables. Check for conflicts among those. // Track dependent and overridable variables. - for (auto &pair : packages) { + for (auto &pair : packages.AllPackages()) { + auto &name = pair.first; auto &package = pair.second; package->ValidateMetadata(); // set unset flags // sort - var_tracker.CategorizeCollection(package->label(), package->AllFields(), - &cvar_provider); + var_tracker.CategorizeCollection(name, package->AllFields(), &cvar_provider); for (auto &p2 : package->AllSparseFields()) { // sparse auto &var = p2.first; auto &mdict = p2.second; for (auto &p3 : mdict) { auto &metadata = p3.second; - var_tracker.Categorize(package->label(), var, metadata, &sparse_provider); + var_tracker.Categorize(name, var, metadata, &sparse_provider); } } - swarm_tracker.CategorizeCollection(package->label(), package->AllSwarms(), - &swarm_provider); + swarm_tracker.CategorizeCollection(name, package->AllSwarms(), &swarm_provider); } // check that dependent variables are provided somewhere diff --git a/src/interface/state_descriptor.hpp b/src/interface/state_descriptor.hpp index 48b80bfeced4..3da8afc7c5a1 100644 --- a/src/interface/state_descriptor.hpp +++ b/src/interface/state_descriptor.hpp @@ -26,6 +26,7 @@ #include "interface/params.hpp" #include "interface/swarm.hpp" #include "refinement/amr_criteria.hpp" +#include "utils/error_checking.hpp" namespace parthenon { @@ -245,7 +246,26 @@ class StateDescriptor { Dictionary> swarmValueMetadataMap_; }; -using Packages_t = Dictionary>; +class Packages_t { + public: + Packages_t() = default; + void Add(const std::shared_ptr &package) { + const auto &name = package->label(); + PARTHENON_REQUIRE_THROWS(packages_.count(name) == 0, + "Package name " + name + " must be unique."); + packages_[name] = package; + return; + } + std::shared_ptr const &Get(const std::string &name) { + return packages_.at(name); + } + const Dictionary> &AllPackages() const { + return packages_; + } + + private: + Dictionary> packages_; +}; std::shared_ptr ResolvePackages(Packages_t &packages); diff --git a/src/interface/update.hpp b/src/interface/update.hpp index 49dd7797b97d..aa47c50e05cc 100644 --- a/src/interface/update.hpp +++ b/src/interface/update.hpp @@ -85,7 +85,7 @@ template TaskStatus EstimateTimestep(T *rc) { Kokkos::Profiling::pushRegion("Task_EstimateTimestep"); Real dt_min = std::numeric_limits::max(); - for (const auto &pkg : rc->GetParentPointer()->packages) { + for (const auto &pkg : rc->GetParentPointer()->packages.AllPackages()) { Real dt = pkg.second->EstimateTimestep(rc); dt_min = std::min(dt_min, dt); } @@ -99,17 +99,17 @@ TaskStatus FillDerived(T *rc) { Kokkos::Profiling::pushRegion("Task_FillDerived"); auto pm = rc->GetParentPointer(); Kokkos::Profiling::pushRegion("PreFillDerived"); - for (const auto &pkg : pm->packages) { + for (const auto &pkg : pm->packages.AllPackages()) { pkg.second->PreFillDerived(rc); } Kokkos::Profiling::popRegion(); // PreFillDerived Kokkos::Profiling::pushRegion("FillDerived"); - for (const auto &pkg : pm->packages) { + for (const auto &pkg : pm->packages.AllPackages()) { pkg.second->FillDerived(rc); } Kokkos::Profiling::popRegion(); // FillDerived Kokkos::Profiling::pushRegion("PostFillDerived"); - for (const auto &pkg : pm->packages) { + for (const auto &pkg : pm->packages.AllPackages()) { pkg.second->PostFillDerived(rc); } Kokkos::Profiling::popRegion(); // PostFillDerived diff --git a/src/parthenon_manager.cpp b/src/parthenon_manager.cpp index 6d5349d881d0..d72294dcf60b 100644 --- a/src/parthenon_manager.cpp +++ b/src/parthenon_manager.cpp @@ -131,7 +131,7 @@ ParthenonStatus ParthenonManager::ParthenonInit(int argc, char *argv[]) { // set up all the packages in the application auto packages = ProcessPackages(pinput); // always add the Refinement package - packages["ParthenonRefinement"] = Refinement::Initialize(pinput.get()); + packages.Add(Refinement::Initialize(pinput.get())); if (arg.res_flag == 0) { pmesh = std::make_unique(pinput.get(), app_input.get(), properties, packages, @@ -157,7 +157,7 @@ ParthenonStatus ParthenonManager::ParthenonInit(int argc, char *argv[]) { } // add root_level to all max_level - for (auto const &ph : packages) { + for (auto const &ph : packages.AllPackages()) { for (auto &amr : ph.second->amr_criteria) { amr->max_level += pmesh->GetRootLevel(); } diff --git a/src/pgen/default_pgen.cpp b/src/pgen/default_pgen.cpp index dc8c6a79b8c1..f7062d425374 100644 --- a/src/pgen/default_pgen.cpp +++ b/src/pgen/default_pgen.cpp @@ -56,14 +56,14 @@ void Mesh::UserWorkInLoopDefault(Mesh *, ParameterInput *, SimTime const &) { void Mesh::PreStepUserDiagnosticsInLoopDefault(Mesh *pmesh, ParameterInput *, SimTime const &simtime) { - for (auto &package : pmesh->packages) { + for (auto &package : pmesh->packages.AllPackages()) { package.second->PreStepDiagnostics(simtime, pmesh->mesh_data.Get().get()); } } void Mesh::PostStepUserDiagnosticsInLoopDefault(Mesh *pmesh, ParameterInput *, SimTime const &simtime) { - for (auto &package : pmesh->packages) { + for (auto &package : pmesh->packages.AllPackages()) { package.second->PostStepDiagnostics(simtime, pmesh->mesh_data.Get().get()); } } diff --git a/src/refinement/refinement.cpp b/src/refinement/refinement.cpp index b0785a1d358e..4f0926260822 100644 --- a/src/refinement/refinement.cpp +++ b/src/refinement/refinement.cpp @@ -62,7 +62,7 @@ AmrTag CheckAllRefinement(MeshBlockData *rc) { std::shared_ptr pmb = rc->GetBlockPointer(); // delta_level holds the max over all criteria. default to derefining. AmrTag delta_level = AmrTag::derefine; - for (auto &pkg : pmb->packages) { + for (auto &pkg : pmb->packages.AllPackages()) { auto &desc = pkg.second; delta_level = std::max(delta_level, desc->CheckRefinement(rc)); if (delta_level == AmrTag::refine) { diff --git a/tst/unit/test_state_descriptor.cpp b/tst/unit/test_state_descriptor.cpp index cd090bf7af4a..8bbae73dbfab 100644 --- a/tst/unit/test_state_descriptor.cpp +++ b/tst/unit/test_state_descriptor.cpp @@ -33,6 +33,28 @@ using parthenon::ResolvePackages; using parthenon::StateDescriptor; using FlagVec = std::vector; +TEST_CASE("Test Add/Get in Packages_t", "[Packages_t]") { + GIVEN("A Packages_t object and a few packages") { + Packages_t packages; + auto pkg1 = std::make_shared("package1"); + auto pkg2 = std::make_shared("package2"); + THEN("We can add a package") { + packages.Add(pkg1); + AND_THEN("The package is available and named correctly") { + auto &pkg = packages.Get("package1"); + REQUIRE(pkg->label() == "package1"); + } + AND_THEN("Requesting a package not added throws an error") { + REQUIRE_THROWS(packages.Get("package2")); + } + AND_THEN("Adding a different package with the same name throws an error") { + auto pkg3 = std::make_shared("package1"); + REQUIRE_THROWS(packages.Add(pkg3)); + } + } + } +} + TEST_CASE("Test Associate in StateDescriptor", "[StateDescriptor]") { GIVEN("Some flags and state descriptors") { FlagVec foo = {Metadata::Independent, Metadata::FillGhost}; @@ -87,9 +109,9 @@ TEST_CASE("Test reqendency resolution in StateDescriptor", "[StateDescriptor]") auto pkg1 = std::make_shared("package1"); auto pkg2 = std::make_shared("package2"); auto pkg3 = std::make_shared("package3"); - packages["package1"] = pkg1; - packages["package2"] = pkg2; - packages["package3"] = pkg3; + packages.Add(pkg1); + packages.Add(pkg2); + packages.Add(pkg3); WHEN("We add metadata with a sparse ID but the sparse flag unset") { THEN("We raise an error") {