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.

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: Bounds in output for Metadata::None variables #1188

Merged
merged 9 commits into from
Nov 13, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- [[PR 1172]](https://github.com/parthenon-hpc-lab/parthenon/pull/1172) Make parthenon manager robust against external MPI init and finalize calls

### Fixed (not changing behavior/API/variables/...)
- [[PR 1188]](https://github.com/parthenon-hpc-lab/parthenon/pull/1188) Fix hdf5 output issue for metadata none variables, update test.
- [[PR 1178]](https://github.com/parthenon-hpc-lab/parthenon/pull/1178) Fix issue with mesh pointer when using relative residual tolerance in BiCGSTAB solver.
- [[PR 1173]](https://github.com/parthenon-hpc-lab/parthenon/pull/1173) Make debugging easier by making parthenon throw an error if ParameterInput is different on multiple MPI ranks.

Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ include(cmake/Format.cmake)
include(cmake/Lint.cmake)

# regression test reference data
set(REGRESSION_GOLD_STANDARD_VER 24 CACHE STRING "Version of gold standard to download and use")
set(REGRESSION_GOLD_STANDARD_VER 25 CACHE STRING "Version of gold standard to download and use")
set(REGRESSION_GOLD_STANDARD_HASH
"SHA512=e220df92a335131131e42ddb52dc221a6dbd6bb56361483b4af0292620eeb82ffb21ef3b95fd9a7c5cc158fb754da0bf1a1015bec98b5bbad05f4bceb1ee99bc"
"SHA512=1e83445b117c5062cc33f3ec65ed70a2057f945c64aa4ea2ee9fa321716b931dec8dbc068376df60e84b4336f03c4204438938fdc09c72794a02597654298d65"
CACHE STRING "Hash of default gold standard file to download")
option(REGRESSION_GOLD_STANDARD_SYNC "Automatically sync gold standard files." ON)

Expand Down
34 changes: 34 additions & 0 deletions example/advection/advection_package.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,20 @@ std::shared_ptr<StateDescriptor> Initialize(ParameterInput *pin) {
m = Metadata({Metadata::Cell, Metadata::OneCopy}, std::vector<int>({1}));
pkg->AddField("my_derived_var", m);

// Create a Metadata::None variable for IO testing purposes.
// Only load if test_metadata_none is specified in the Advection block
auto test_metadata_none =
pin->GetOrAddBoolean("Advection", "test_metadata_none", false);
pkg->AddParam<bool>("test_metadata_none", test_metadata_none);
if (test_metadata_none) {
const int nx1 = pin->GetOrAddInteger("parthenon/mesh", "nx1", 1);
const int nx2 = pin->GetOrAddInteger("parthenon/mesh", "nx2", 1);
const int nx3 = pin->GetOrAddInteger("parthenon/mesh", "nx3", 1);
std::vector<int> test_shape = {nx1 + 1, nx2 + 1, nx3 + 1, 3};
m = Metadata({Metadata::OneCopy, Metadata::None}, test_shape);
pkg->AddField("metadata_none_var", m);
}

// List (vector) of HistoryOutputVar that will all be enrolled as output variables
parthenon::HstVar_list hst_vars = {};
// Now we add a couple of callback functions
Expand Down Expand Up @@ -281,6 +295,7 @@ AmrTag CheckRefinement(MeshBlockData<Real> *rc) {
void PreFill(MeshBlockData<Real> *rc) {
auto pmb = rc->GetBlockPointer();
auto pkg = pmb->packages.Get("advection_package");
const bool test_metadata_none = pkg->Param<bool>("test_metadata_none");
bool fill_derived = pkg->Param<bool>("fill_derived");

if (fill_derived) {
Expand All @@ -302,6 +317,25 @@ void PreFill(MeshBlockData<Real> *rc) {
v(out + n, k, j, i) = 1.0 - v(in + n, k, j, i);
});
}

// Fill the metadata::None var with index gymnastics.
if (test_metadata_none) {
IndexRange ib = pmb->cellbounds.GetBoundsI(IndexDomain::interior);
IndexRange jb = pmb->cellbounds.GetBoundsJ(IndexDomain::interior);
IndexRange kb = pmb->cellbounds.GetBoundsK(IndexDomain::interior);

// packing in principle unnecessary/convoluted here and just done for demonstration
std::vector<std::string> vars({"metadata_none_var"});
PackIndexMap imap;
const auto &v = rc->PackVariables(vars, imap);

const int ivar = imap.get("metadata_none_var").first;
pmb->par_for(
PARTHENON_AUTO_LABEL, 0, 2, kb.s, kb.e, jb.s, jb.e, ib.s, ib.e,
KOKKOS_LAMBDA(const int n, const int k, const int j, const int i) {
v(ivar, n, k, j, i) = n + k * j * i;
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what/why this is happening here.
If I read the the field correctly, then it's a vector with 3 components and extents larger than the mesh size by one.
In this par_for the block indices are used (without any global offset), so every block will have the same indices set to the same values and a large fraction of the index space will remain 0.
Is that intentional?

Copy link
Collaborator Author

@AstroBarker AstroBarker Oct 21, 2024

Choose a reason for hiding this comment

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

Good catch, I didn't pay much mind to the contents of the fields, only that its shape reproduced the crash. This should be fixed to be filled now, I think.

}

// this is the package registered function to fill derived
Expand Down
6 changes: 3 additions & 3 deletions src/outputs/output_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,11 @@ void PackOrUnpackVar(const VarInfo &info, bool do_ghosts, idx_t &idx, Function_t
auto [kb, jb, ib] = info.GetPaddedBoundsKJI(domain);
if (info.where == MetadataFlag({Metadata::None})) {
kb.s = 0;
kb.e = shape[4];
kb.e = std::max(0, shape[4] - 1);
jb.s = 0;
jb.e = shape[5];
jb.e = std::max(0, shape[5] - 1);
ib.s = 0;
ib.e = shape[6];
ib.e = std::max(0, shape[6] - 1);
}
for (int topo = 0; topo < shape[0]; ++topo) {
for (int t = 0; t < shape[1]; ++t) {
Expand Down
4 changes: 3 additions & 1 deletion tst/regression/test_suites/output_hdf5/parthinput.advection
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ vx = 1.0
vy = 1.0
vz = 1.0
profile = hard_sphere
test_metadata_none = true

refine_tol = 0.3 # control the package specific refinement tagging function
derefine_tol = 0.03
Expand All @@ -64,7 +65,8 @@ file_type = hdf5
dt = 1.0
variables = advected, one_minus_advected, & # comments are ok
one_minus_advected_sq, & # on every (& characters are ok in comments)
one_minus_sqrt_one_minus_advected_sq # line
one_minus_sqrt_one_minus_advected_sq, & # line
metadata_none_var
pgrete marked this conversation as resolved.
Show resolved Hide resolved

<parthenon/output1>
file_type = hst
Expand Down
Loading