Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix meshblock pack issues coming from template shadowing and add a test #572

Merged
merged 3 commits into from
Jul 26, 2021
Merged

fix meshblock pack issues coming from template shadowing and add a test #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac70d29
fix meshblock pack issues coming from template shadowing and add a te…
jonahm-LANL Jul 23, 2021
595713f
CHANGELOG
jonahm-LANL Jul 23, 2021
96d1257
Merge branch 'develop' into jmm/fix-meshblock-pack
Yurlungur Jul 26, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
- [[PR 522]](https://github.com/lanl/parthenon/pull/522) Corrected ordering of `OutputDatasetNames` to match `ComponentNames`

### Fixed (not changing behavior/API/variables/...)
- [[PR 572]](https://github.com/lanl/parthenon/pull/572) Fix meshblockpack issue coming from variatic template shadowing
- [[PR 569]](https://github.com/lanl/parthenon/pull/569) Fix path to nvcc_wrapper in README example
- [[PR 551]](https://github.com/lanl/parthenon/pull/551) Hotfix to make particles compile without MPI again
- [[PR 552]](https://github.com/lanl/parthenon/pull/552) Fix missing include for fstream
Expand Down
9 changes: 8 additions & 1 deletion example/calculate_pi/calculate_pi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,17 @@ TaskStatus ComputeArea(std::shared_ptr<MeshData<Real>> &md, ParArrayHost<Real> a
bool const use_sparse =
md->GetMeshPointer()->packages.Get("calculate_pi")->Param<bool>("use_sparse");

PackIndexMap imap; // PackIndex map can be used to get the index in
// a pack of a specific variable
std::vector<std::string> key; // The key is a hash used for caching
// packs. You shouldn't need to use
// it, but it is accessible.
// This call signature works
const auto &pack = use_sparse
? md->PackVariables(std::vector<std::string>({"in_or_out"}),
std::vector<int>{0})
std::vector<int>{0}, imap, key)

// and so does this one
: md->PackVariables(std::vector<std::string>({"in_or_out"}));

areas(i) = use_sparse ? ComputeAreaInternal(
Expand Down
223 changes: 192 additions & 31 deletions src/interface/mesh_data.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,50 +289,211 @@ class MeshData {

template <typename... Args>
const auto &PackVariablesImpl(PackIndexMap *map_out, std::vector<std::string> *key_out,
Args &&... args) {
bool coarse, Args &&... args) {
auto pack_function = [&](std::shared_ptr<MeshBlockData<T>> meshblock_data,
PackIndexMap &map, std::vector<std::string> &key) {
return meshblock_data->PackVariables(std::forward<Args>(args)..., map, key);
return meshblock_data->PackVariables(std::forward<Args>(args)..., map, key, coarse);
};
return pack_on_mesh_impl::PackOnMesh<VariablePack<T>>(
varPackMap_, block_data_, pack_function, map_out, key_out);
}

public:
template <typename... Args>
const auto &PackVariablesAndFluxes(Args &&... args, PackIndexMap &map,
// DO NOT use variatic templates here. They shadow each other

// Pack by separate variable and flux names
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
const std::vector<int> &sparse_ids,
PackIndexMap &map, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key, var_names, flx_names, sparse_ids);
}
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
const std::vector<int> &sparse_ids,
PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr, var_names, flx_names, sparse_ids);
}
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
const std::vector<int> &sparse_ids,
vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key, std::forward<Args>(args)...);
}
template <typename... Args>
const auto &PackVariablesAndFluxes(Args &&... args, PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr, std::forward<Args>(args)...);
}
template <typename... Args>
const auto &PackVariablesAndFluxes(Args &&... args, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(nullptr, &key, std::forward<Args>(args)...);
}
template <typename... Args>
const auto &PackVariablesAndFluxes(Args &&... args) {
return PackVariablesAndFluxesImpl(nullptr, nullptr, std::forward<Args>(args)...);
return PackVariablesAndFluxesImpl(nullptr, &key, var_names, flx_names, sparse_ids);
}
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
const std::vector<int> &sparse_ids) {
return PackVariablesAndFluxesImpl(nullptr, nullptr, var_names, flx_names, sparse_ids);
}
// no sparse ids
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
PackIndexMap &map, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key, var_names, flx_names);
}
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr, var_names, flx_names);
}
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names,
vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(nullptr, &key, var_names, flx_names);
}
const auto &PackVariablesAndFluxes(const std::vector<std::string> &var_names,
const std::vector<std::string> &flx_names) {
return PackVariablesAndFluxesImpl(nullptr, nullptr, var_names, flx_names);
}
// Pack by either the same variable and flux names, or by metadata flags
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
const std::vector<int> &sparse_ids,
PackIndexMap &map, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key, names_or_flags, sparse_ids);
}
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
const std::vector<int> &sparse_ids,
vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(nullptr, &key, names_or_flags, sparse_ids);
}
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
const std::vector<int> &sparse_ids,
PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr, names_or_flags, sparse_ids);
}
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
const std::vector<int> &sparse_ids) {
return PackVariablesAndFluxesImpl(nullptr, nullptr, names_or_flags, sparse_ids);
}
// no sparse ids
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
PackIndexMap &map, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key, names_or_flags);
}
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(nullptr, &key, names_or_flags);
}

template <typename... Args>
const auto &PackVariables(Args &&... args, PackIndexMap &map,
std::vector<std::string> &key) {
return PackVariablesImpl(&map, &key, std::forward<Args>(args)...);
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr, names_or_flags);
}
template <typename... Args>
const auto &PackVariables(Args &&... args, PackIndexMap &map) {
return PackVariablesImpl(&map, nullptr, std::forward<Args>(args)...);
template <typename Elem>
const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags) {
return PackVariablesAndFluxesImpl(nullptr, nullptr, names_or_flags);
}
template <typename... Args>
const auto &PackVariables(Args &&... args, std::vector<std::string> &key) {
return PackVariablesImpl(nullptr, &key, std::forward<Args>(args)...);
// only sparse ids
const auto &PackVariablesAndFluxes(const std::vector<int> &sparse_ids,
PackIndexMap &map, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key, sparse_ids);
}
template <typename... Args>
const auto &PackVariables(Args &&... args) {
return PackVariablesImpl(nullptr, nullptr, std::forward<Args>(args)...);
const auto &PackVariablesAndFluxes(const std::vector<int> &sparse_ids,
vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(nullptr, &key, sparse_ids);
}
const auto &PackVariablesAndFluxes(const std::vector<int> &sparse_ids,
PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr, sparse_ids);
}
const auto &PackVariablesAndFluxes(const std::vector<int> &sparse_ids) {
return PackVariablesAndFluxesImpl(nullptr, nullptr, sparse_ids);
}
Comment on lines +397 to +407
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these actually required?
If I'm not missing something they're matching the

  const auto &PackVariablesAndFluxes(const std::vector<Elem> &names_or_flags,
                                     PackIndexMap &map, vpack_types::StringPair &key)

above, aren't they?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These exclude either the PackIndexMap or the key, or both. Whereas the one you quoted has both of them. They're just syntactic sugar overloads.

// No nothing
const auto &PackVariablesAndFluxes(PackIndexMap &map, vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(&map, &key);
}
const auto &PackVariablesAndFluxes(vpack_types::StringPair &key) {
return PackVariablesAndFluxesImpl(nullptr, &key);
}
const auto &PackVariablesAndFluxes(PackIndexMap &map) {
return PackVariablesAndFluxesImpl(&map, nullptr);
}
const auto &PackVariablesAndFluxes() {
return PackVariablesAndFluxesImpl(nullptr, nullptr);
}
brryan marked this conversation as resolved.
Show resolved Hide resolved

// As above, no variatics because they shadow each other.
// Now the pack variables overloads.
// covers names and metadata flags
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags,
const std::vector<int> &sparse_ids, PackIndexMap &map,
std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(&map, &key, coarse, names_or_flags, sparse_ids);
}
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags,
const std::vector<int> &sparse_ids, PackIndexMap &map,
bool coarse = false) {
return PackVariablesImpl(&map, nullptr, coarse, names_or_flags, sparse_ids);
}
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags,
const std::vector<int> &sparse_ids,
std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(nullptr, &key, coarse, names_or_flags, sparse_ids);
}
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags,
const std::vector<int> &sparse_ids, bool coarse = false) {
return PackVariablesImpl(nullptr, nullptr, coarse, names_or_flags, sparse_ids);
}
// no sparse ids
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags, PackIndexMap &map,
std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(&map, &key, coarse, names_or_flags);
}
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags, PackIndexMap &map,
bool coarse = false) {
return PackVariablesImpl(&map, nullptr, coarse, names_or_flags);
}
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags,
std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(nullptr, &key, coarse, names_or_flags);
}
template <typename Elem>
const auto &PackVariables(const std::vector<Elem> names_or_flags, bool coarse = false) {
return PackVariablesImpl(nullptr, nullptr, coarse, names_or_flags);
}
// No names or flags
const auto &PackVariables(const std::vector<int> &sparse_ids, PackIndexMap &map,
std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(&map, &key, coarse, sparse_ids);
}
const auto &PackVariables(const std::vector<int> &sparse_ids, PackIndexMap &map,
bool coarse = false) {
return PackVariablesImpl(&map, nullptr, coarse, sparse_ids);
}
const auto &PackVariables(const std::vector<int> &sparse_ids,
std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(nullptr, &key, coarse, sparse_ids);
}
const auto &PackVariables(const std::vector<int> &sparse_ids, bool coarse = false) {
return PackVariablesImpl(nullptr, nullptr, coarse, sparse_ids);
}
// no nothing
const auto &PackVariables(PackIndexMap &map, std::vector<std::string> &key,
bool coarse = false) {
return PackVariablesImpl(&map, &key, coarse);
}
const auto &PackVariables(PackIndexMap &map, bool coarse = false) {
return PackVariablesImpl(&map, nullptr, coarse);
}
const auto &PackVariables(std::vector<std::string> &key, bool coarse = false) {
return PackVariablesImpl(nullptr, &key, coarse);
}
const auto &PackVariables(bool coarse = false) {
return PackVariablesImpl(nullptr, nullptr, coarse);
}

void ClearCaches() {
Expand Down