Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@


### Changed (changing behavior/API/variables/...)
- [[PR 1449]](https://github.com/parthenon-hpc-lab/parthenon/pull/1449) Require all containers with the same base name in a DataCollection be made from the same field set
- [[PR 1438]](https://github.com/parthenon-hpc-lab/parthenon/pull/1438) Performance tuning for the loop abstraction machinery and add loop abstraction OpenMP support
- [[PR 1416][(https://github.com/parthenon-hpc-lab/parthenon/pull/1416) Remove virtual tag from destructors in sparse and swarm pack base classes
- [[PR 1401]](https://github.com/parthenon-hpc-lab/parthenon/pull/1401) Sparse Field Component Names
Expand Down Expand Up @@ -68,6 +69,7 @@


### Incompatibilities (i.e. breaking changes)
- [[PR 1449]](https://github.com/parthenon-hpc-lab/parthenon/pull/1449) Require all containers with the same base name in a DataCollection be made from the same field set
- [[PR 1385]](https://github.com/parthenon-hpc-lab/parthenon/pull/1385) ParameterInput internal storage refactor removes direct access to linked list (`pfirst_block`). Use `GetBlocksWithPrefix()` or `GetBlockNames()` instead.
- [[PR 1351]](https://github.com/parthenon-hpc-lab/parthenon/pull/1351) Bump Kokkos 5 & C++20
- [[PR 1377]](https://github.com/parthenon-hpc-lab/parthenon/pull/1377) Extend Initialization Hierarchy
Expand Down
104 changes: 83 additions & 21 deletions src/interface/data_collection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

#include <map>
#include <memory>
#include <set>
#include <stdexcept>
#include <string>
#include <vector>

#include "basic_types.hpp"
#include "globals.hpp"
#include "interface/variable.hpp"
#include "utils/concepts_lite.hpp"
#include "utils/error_checking.hpp"
#include "utils/unique_id.hpp"

namespace parthenon {
class Mesh;
Expand Down Expand Up @@ -53,36 +56,37 @@ class DataCollection {

void SetMeshPointer(Mesh *pmesh) { pmy_mesh_ = pmesh; }

template <class SRC_t, typename ID_t>
std::shared_ptr<T> &Add(const std::string &name, const std::shared_ptr<SRC_t> &src,
const std::vector<ID_t> &fields, const bool shallow) {
auto key = GetKey(name, src);
auto it = containers_.find(key);
if (it != containers_.end()) {
if (fields.size() && !(it->second)->CreatedFrom(fields)) {
PARTHENON_THROW(key + " already exists in collection but fields do not match.");
}
return it->second;
}

auto c = std::make_shared<T>(name);
c->Initialize(src, fields, shallow);

containers_[key] = c;
return containers_[key];
}

template <class SRC_t, typename ID_t = std::string>
std::shared_ptr<T> &Add(const std::string &label, const std::shared_ptr<SRC_t> &src,
const std::vector<ID_t> &fields = {}) {
return Add(label, src, fields, false);
return AddImpl(label, src, fields, false);
}

template <class SRC_t, typename ID_t>
std::shared_ptr<T> &Add(const std::string &label, const std::shared_ptr<SRC_t> &src,
const std::vector<ID_t> &fields, const bool shallow) {
return AddImpl(label, src, fields, shallow);
}

template <class SRC_t, typename ID_t = std::string>
std::shared_ptr<T> &AddShallow(const std::string &label,
const std::shared_ptr<SRC_t> &src,
const std::vector<ID_t> &fields = {}) {
return Add(label, src, fields, true);
return AddImpl(label, src, fields, true);
}

template <class SRC_t, typename ID_t = Uid_t>
std::shared_ptr<T> &AddFromSet(const std::string &label,
const std::shared_ptr<SRC_t> &src,
const std::set<ID_t> &fields) {
return AddImpl(label, src, fields, false);
}

template <class SRC_t, typename ID_t = Uid_t>
std::shared_ptr<T> &AddShallowFromSet(const std::string &label,
const std::shared_ptr<SRC_t> &src,
const std::set<ID_t> &fields) {
return AddImpl(label, src, fields, true);
}

auto &Stages() { return containers_; }
Expand Down Expand Up @@ -112,6 +116,15 @@ class DataCollection {
std::shared_ptr<T> &Get(const std::string &name = "base");
const std::shared_ptr<T> &Get(const std::string &name = "base") const;

// The field list (as a canonical variable-uid set) that the named container was created
// from. Every container sharing a base name is created from the same list (see the
// warning in Add). If the name has never been added, returns a static empty set.
const std::set<Uid_t> &GetCreationFields(const std::string &name) const {
Comment thread
Yurlungur marked this conversation as resolved.
static const std::set<Uid_t> empty;
const auto nit = name_creation_fields_.find(name);
return nit == name_creation_fields_.end() ? empty : nit->second;
}
Comment thread
Yurlungur marked this conversation as resolved.

void Set(const std::string &name, std::shared_ptr<T> &d) { containers_[name] = d; }

// Legacy methods that are specific to MeshData
Expand All @@ -122,6 +135,54 @@ class DataCollection {
void clear() { containers_.clear(); }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should the clear also clear name_creation_fields_?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes, good catch.


private:
template <class SRC_t, class Fields_t>
std::shared_ptr<T> &AddImpl(const std::string &name, const std::shared_ptr<SRC_t> &src,
const Fields_t &fields, const bool shallow) {
auto key = GetKey(name, src);
auto it = containers_.find(key);
if (it != containers_.end()) {
// Existing container. An explicit field list must match what the container was
// actually created from (checked against the container itself, which also catches
// containers built by hand or through a different DataCollection); an empty list
// means "all fields"/"don't check" and always passes.
Comment thread
Yurlungur marked this conversation as resolved.
if (fields.size() && !(it->second)->CreatedFrom(fields))
PARTHENON_THROW(key + " already exists in collection but fields do not match.");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Might be useful for debugging to print the source label (if we know it) and both sets of fields.

return it->second;
}

using ID_t = typename Fields_t::value_type;
auto to_uid = [](const ID_t &f) -> Uid_t {
if constexpr (std::is_same_v<ID_t, std::string>)
return Variable<Real>::GetUniqueID(f);
else
return f;
};

// Track the field list (as a canonical uid set) each container name is created from,
// so the DataCollection is the single source of truth for it (see GetCreationFields).
Comment thread
lroberts36 marked this conversation as resolved.
Outdated
// Containers sharing a base name but built from different sources get distinct keys,
Comment thread
lroberts36 marked this conversation as resolved.
Outdated
// so the per-key CreatedFrom check above cannot compare them; this does. All
Comment thread
lroberts36 marked this conversation as resolved.
Outdated
// instances of a name must be created from the same list -- fail if not.
Comment thread
lroberts36 marked this conversation as resolved.
Outdated
std::set<Uid_t> created;
for (const auto &f : fields)
created.insert(to_uid(f));
auto nit = name_creation_fields_.find(name);
if (nit == name_creation_fields_.end()) {
name_creation_fields_[name] = created;
} else if (nit->second != created) {
PARTHENON_THROW(
"Container \"" + name +
"\" is being created from different field lists on different sources. All "
"instances sharing a name must be created from the same field list.");
}
Comment on lines +173 to +177

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Might be useful for debugging to print the source label (if we know it) and both sets of fields.


std::vector<Uid_t> uids(created.begin(), created.end());
auto c = std::make_shared<T>(name);
c->Initialize(src, uids, shallow);
containers_[key] = c;
return containers_[key];
}

std::string GetKey(const std::string &stage_label,
const std::shared_ptr<BlockListPartition> &in) const;
std::string GetKey(const std::string &stage_label,
Expand All @@ -133,6 +194,7 @@ class DataCollection {

Mesh *pmy_mesh_;
std::map<std::string, std::shared_ptr<T>> containers_;
std::map<std::string, std::set<Uid_t>> name_creation_fields_;
};

} // namespace parthenon
Expand Down
5 changes: 5 additions & 0 deletions src/interface/meshblock_data.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,11 @@ class MeshBlockData {
std::all_of(vars.begin(), vars.end(),
[this](const auto &v) { return this->varUidIn_.count(v); });
}
bool CreatedFrom(const std::set<Uid_t> &vars) {
return (vars.size() == varUidIn_.size()) &&
std::all_of(vars.begin(), vars.end(),
[this](const auto &v) { return this->varUidIn_.count(v); });
}
bool CreatedFrom(const std::vector<std::string> &vars) {
return (vars.size() == varUidIn_.size()) &&
std::all_of(vars.begin(), vars.end(), [this](const auto &v) {
Expand Down
7 changes: 7 additions & 0 deletions src/interface/state_descriptor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "interface/params.hpp"
#include "interface/sparse_pool.hpp"
#include "interface/var_id.hpp"
#include "interface/variable.hpp"
#include "outputs/output_parameters.hpp"
#include "pack/scratch_variables.hpp"
#include "parameter_input.hpp"
Expand Down Expand Up @@ -313,9 +314,15 @@ class StateDescriptor {
const auto &GetFieldVarID(const std::string &label) const {
return labelToVidMap_.at(label);
}
const auto &GetFieldVarID(const Uid_t &uid) const {
return labelToVidMap_.at(Variable<Real>::GetLabel(uid));
}
const auto &GetFieldMetadata(const std::string &label) const {
return metadataMap_.at(labelToVidMap_.at(label));
}
const auto &GetFieldMetadata(const Uid_t &uid) const {
return metadataMap_.at(labelToVidMap_.at(Variable<Real>::GetLabel(uid)));
}
const auto &GetFieldMetadata(const VarID &id) const { return metadataMap_.at(id); }
const auto &AllFields() const noexcept { return metadataMap_; }
const auto &AllSparsePools() const noexcept { return sparsePoolMap_; }
Expand Down
8 changes: 6 additions & 2 deletions src/solvers/bicgstab_solver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ class BiCGSTABSolver : public SolverBase, BiCGSTABSolverCounter {
return preconditioner.AddSetupTasks(tl, dependence, partition, pmesh);
} else if (params_.precondition_type == Preconditioner::Diagonal) {
auto partitions = pmesh->GetDefaultBlockPartitions();
auto &md = pmesh->mesh_data.Add(container_base, partitions[partition]);
auto &md =
pmesh->mesh_data.AddFromSet(container_base, partitions[partition],
pmesh->mesh_data.GetCreationFields(container_base));
auto &md_diag = pmesh->mesh_data.Add(container_diag, md, sol_fields);
return tl.AddTask(dependence, &equations_t::SetDiagonal, &eqs_, md, md_diag);
} else {
Expand All @@ -161,7 +163,9 @@ class BiCGSTABSolver : public SolverBase, BiCGSTABSolverCounter {
auto partitions = pmesh->GetDefaultBlockPartitions();
// Should contain all fields necessary for applying the matrix to a give state vector,
// e.g. diffusion coefficients and diagonal, these will not be modified by the solvers
auto &md_base = pmesh->mesh_data.Add(container_base, partitions[partition]);
auto &md_base =
pmesh->mesh_data.AddFromSet(container_base, partitions[partition],
pmesh->mesh_data.GetCreationFields(container_base));
// Container in which the solution is stored and with which the downstream user can
Comment on lines +166 to 169

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this imply that this is the recommend downstream pattern now, too?
If so, it might be worth to also mention this in the doc (and in general add a two-liner in the existing doc to reflect this change).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, I don't think this is the generally recommended pattern unless you need to do something specifically like what is done in multigrid where someone creates containers in one place from a set of partitions/meshdata and then somewhere else you need to create them from a different set of partitions/meshdata.

// interact. This container only requires the fields in sol_fields
auto &md_u = pmesh->mesh_data.Add(container_u, partitions[partition], sol_fields);
Expand Down
12 changes: 8 additions & 4 deletions src/solvers/mg_solver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ class MGSolver : public SolverBase, MGSolverCounter {
PARTHENON_FAIL("Does not work with non-default partitioning.");
auto partition = partitions[default_partition_idx];

auto &md = pmesh->mesh_data.Add(container_base, partition);
auto &md = pmesh->mesh_data.AddFromSet(
container_base, partition, pmesh->mesh_data.GetCreationFields(container_base));
auto &md_u = pmesh->mesh_data.Add(container_u, partition, sol_fields);
auto &md_res_err = pmesh->mesh_data.Add(container_res_err, partition, sol_fields);
auto &md_rhs = pmesh->mesh_data.Add(container_rhs, partition, sol_fields);
Expand Down Expand Up @@ -340,7 +341,8 @@ class MGSolver : public SolverBase, MGSolverCounter {
bool input_is_zero) {
using namespace utils;

auto &md_base = pmesh->mesh_data.Add(container_base, partition);
auto &md_base = pmesh->mesh_data.AddFromSet(
container_base, partition, pmesh->mesh_data.GetCreationFields(container_base));
auto &md_rhs = pmesh->mesh_data.Add(container_rhs, partition, sol_fields);
auto &md_diag = pmesh->mesh_data.Add(container_diag, partition, sol_fields);
auto &md_ax = pmesh->mesh_data.Add(container_temp, partition, sol_fields);
Expand Down Expand Up @@ -418,7 +420,8 @@ class MGSolver : public SolverBase, MGSolverCounter {
const int level = partition->grid.multigrid_level();
const auto [min_level, max_level] = GetMinMaxLevel(pmesh);

auto &md = pmesh->mesh_data.Add(container_base, partition);
auto &md = pmesh->mesh_data.AddFromSet(
container_base, partition, pmesh->mesh_data.GetCreationFields(container_base));
auto &md_diag = pmesh->mesh_data.Add(container_diag, partition, sol_fields);

auto task_out = dependence;
Expand Down Expand Up @@ -457,7 +460,8 @@ class MGSolver : public SolverBase, MGSolverCounter {

bool do_FAS = params_.do_FAS;

auto &md = pmesh->mesh_data.Add(container_base, partition);
auto &md = pmesh->mesh_data.AddFromSet(
container_base, partition, pmesh->mesh_data.GetCreationFields(container_base));
auto &md_u = pmesh->mesh_data.Add(container_u, partition, sol_fields);
auto &md_rhs = pmesh->mesh_data.Add(container_rhs, partition, sol_fields);
auto &md_res_err = pmesh->mesh_data.Add(container_res_err, partition, sol_fields);
Expand Down
Loading