Remove ambiguity in package names.

CHANGELOG.md

@@ -6,6 +6,7 @@
 - [[PR 412]](https://github.com/lanl/parthenon/pull/412) Add capability to use host (pinned) memory for communication buffers (via PARTHENON_ENABLE_HOST_COMM_BUFFERS - default OFF)
 
 ### 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`.
 
 ### Fixed (not changing behavior/API/variables/...)
 

example/advection/advection_package.cpp

@@ -191,7 +191,7 @@ AmrTag CheckRefinement(MeshBlockData<Real> *rc) {
       },
       Kokkos::MinMax<Real>(minmax));
 
-  auto pkg = pmb->packages["advection_package"];
+  auto pkg = pmb->packages.Get("advection_package");
   const auto &refine_tol = pkg->Param<Real>("refine_tol");
   const auto &derefine_tol = pkg->Param<Real>("derefine_tol");
 
@@ -272,7 +272,7 @@ void PostFill(MeshBlockData<Real> *rc) {
 // provide the routine that estimates a stable timestep for this package
 Real EstimateTimestepBlock(MeshBlockData<Real> *rc) {
   auto pmb = rc->GetBlockPointer();
-  auto pkg = pmb->packages["advection_package"];
+  auto pkg = pmb->packages.Get("advection_package");
   const auto &cfl = pkg->Param<Real>("cfl");
   const auto &vx = pkg->Param<Real>("vx");
   const auto &vy = pkg->Param<Real>("vy");
@@ -312,7 +312,7 @@ TaskStatus CalculateFluxes(std::shared_ptr<MeshBlockData<Real>> &rc) {
   IndexRange kb = pmb->cellbounds.GetBoundsK(IndexDomain::interior);
 
   ParArrayND<Real> advected = rc->Get("advected").data;
-  auto pkg = pmb->packages["advection_package"];
+  auto pkg = pmb->packages.Get("advection_package");
   const auto &vx = pkg->Param<Real>("vx");
   const auto &vy = pkg->Param<Real>("vy");
   const auto &vz = pkg->Param<Real>("vz");

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 &amp = pkg->Param<Real>("amp");
   const auto &vel = pkg->Param<Real>("vel");
   const auto &k_par = pkg->Param<Real>("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 &amp = pkg->Param<Real>("amp");
@@ -212,12 +212,12 @@ void UserWorkAfterLoop(Mesh *mesh, ParameterInput *pin, SimTime &tm) {
 Packages_t ProcessPackages(std::unique_ptr<ParameterInput> &pin) {
   Packages_t packages;
   auto pkg = advection_package::Initialize(pin.get());
-  packages[pkg->label()] = pkg;
+  packages.Add(pkg);
 
   auto app = std::make_shared<StateDescriptor>("advection_app");
   app->PreFillDerivedBlock = advection_package::PreFill;
   app->PostFillDerivedBlock = advection_package::PostFill;
-  packages[app->label()] = app;
+  packages.Add(app);
 
   return packages;
 }

example/calculate_pi/calculate_pi.cpp

@@ -41,7 +41,7 @@ void SetInOrOut(MeshBlockData<Real> *rc) {
   IndexRange jb = pmb->cellbounds.GetBoundsJ(IndexDomain::interior);
   IndexRange kb = pmb->cellbounds.GetBoundsK(IndexDomain::interior);
   ParArrayND<Real> &v = rc->Get("in_or_out").data;
-  const auto &radius = pmb->packages["calculate_pi"]->Param<Real>("radius");
+  const auto &radius = pmb->packages.Get("calculate_pi")->Param<Real>("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<MeshData<Real>> &md, ParArrayHost<Real> a
 }
 
 TaskStatus AccumulateAreas(ParArrayHost<Real> areas, Packages_t &packages) {
-  const auto &radius = packages["calculate_pi"]->Param<Real>("radius");
+  const auto &radius = packages.Get("calculate_pi")->Param<Real>("radius");
 
   Real area = 0.0;
   for (int i = 0; i < areas.GetSize(); i++) {
@@ -119,7 +119,7 @@ TaskStatus AccumulateAreas(ParArrayHost<Real> 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;
 }
 

example/calculate_pi/pi_driver.cpp

@@ -67,7 +67,7 @@ int main(int argc, char *argv[]) {
 Packages_t ProcessPackages(std::unique_ptr<ParameterInput> &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<Real>("pi_val");
+  auto &pi_val = pmesh->packages.Get("calculate_pi")->Param<Real>("pi_val");
   pmesh->mbcnt = pmesh->nbtotal; // this is how many blocks were processed
   PostExecute(pi_val);
   return DriverStatus::complete;

example/face_fields/face_fields_example.cpp

@@ -57,7 +57,7 @@ Packages_t ProcessPackages(std::unique_ptr<ParameterInput> &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<Real>("px");
   Real py = example->Param<Real>("py");
   Real pz = example->Param<Real>("pz");

example/particles/particles.cpp

@@ -27,7 +27,7 @@ namespace particles_example {
 
 Packages_t ProcessPackages(std::unique_ptr<ParameterInput> &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<Real> *rc) { return AmrTag::same; }
 
 Real EstimateTimestepBlock(MeshBlockData<Real> *rc) {
   auto pmb = rc->GetBlockPointer();
-  auto pkg = pmb->packages["particles_package"];
+  auto pkg = pmb->packages.Get("particles_package");
   const Real &dt = pkg->Param<Real>("const_dt");
   return dt;
 }
@@ -97,7 +97,7 @@ Real EstimateTimestepBlock(MeshBlockData<Real> *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<RNGPool>("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<RNGPool>("rng_pool");
   auto num_particles = pkg->Param<int>("num_particles");

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<StateDescriptor> 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

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<Dictionary<Metadata>> swarmValueMetadataMap_;
 };
 
-using Packages_t = Dictionary<std::shared_ptr<StateDescriptor>>;
+class Packages_t {
+ public:
+  Packages_t() = default;
+  void Add(const std::shared_ptr<StateDescriptor> &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<StateDescriptor> const &Get(const std::string &name) {
+    return packages_.at(name);
+  }
+  const Dictionary<std::shared_ptr<StateDescriptor>> &AllPackages() const {
+    return packages_;
+  }
+
+ private:
+  Dictionary<std::shared_ptr<StateDescriptor>> packages_;
+};
 
 std::shared_ptr<StateDescriptor> ResolvePackages(Packages_t &packages);
 

src/interface/update.hpp

@@ -85,7 +85,7 @@ template <typename T>
 TaskStatus EstimateTimestep(T *rc) {
   Kokkos::Profiling::pushRegion("Task_EstimateTimestep");
   Real dt_min = std::numeric_limits<Real>::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

src/parthenon_manager.cpp

@@ -117,7 +117,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<Mesh>(pinput.get(), app_input.get(), properties, packages,
@@ -143,7 +143,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();
     }

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());
   }
 }

src/refinement/refinement.cpp

@@ -62,7 +62,7 @@ AmrTag CheckAllRefinement(MeshBlockData<Real> *rc) {
   std::shared_ptr<MeshBlock> 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) {

tst/unit/test_state_descriptor.cpp

@@ -33,6 +33,28 @@ using parthenon::ResolvePackages;
 using parthenon::StateDescriptor;
 using FlagVec = std::vector<MetadataFlag>;
 
+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<StateDescriptor>("package1");
+    auto pkg2 = std::make_shared<StateDescriptor>("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<StateDescriptor>("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<StateDescriptor>("package1");
     auto pkg2 = std::make_shared<StateDescriptor>("package2");
     auto pkg3 = std::make_shared<StateDescriptor>("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") {