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 bug in static refinement #1071

Merged
merged 10 commits into from
May 15, 2024
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 1004]](https://github.com/parthenon-hpc-lab/parthenon/pull/1004) Allow parameter modification from an input file for restarts

### Fixed (not changing behavior/API/variables/...)
- [[PR 1071]](https://github.com/parthenon-hpc-lab/parthenon/pull/1070) Fix bug in static mesh refinement related to redefinition of Mesh::root_level
- [[PR 1070]](https://github.com/parthenon-hpc-lab/parthenon/pull/1070) Correctly exclude flux vars from searches by default
- [[PR 1049]](https://github.com/parthenon-hpc-lab/parthenon/pull/1049) Catch task failures from threads
- [[PR 1058]](https://github.com/parthenon-hpc-lab/parthenon/pull/1058) Vector history not being output if no scalar history present
Expand Down
2 changes: 1 addition & 1 deletion src/mesh/forest/forest.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Forest {

public:
int root_level;
int forest_level;
std::optional<int> forest_level{};

void AddTree(const std::shared_ptr<Tree> &in) {
if (trees.count(in->GetId())) {
Expand Down
8 changes: 4 additions & 4 deletions src/mesh/mesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ Mesh::Mesh(ParameterInput *pin, ApplicationInput *app_in, Packages_t &packages,
ref_size.xmax(X3DIR) = mesh_size.xmax(X3DIR);
}
int ref_lev = pin->GetInteger(pib->block_name, "level");
int lrlev = ref_lev + root_level;
int lrlev = ref_lev + GetLegacyTreeRootLevel();
// range check
if (ref_lev < 1) {
msg << "### FATAL ERROR in Mesh constructor" << std::endl
Expand Down Expand Up @@ -324,9 +324,9 @@ Mesh::Mesh(ParameterInput *pin, ApplicationInput *app_in, Packages_t &packages,
for (auto dir : {X1DIR, X2DIR, X3DIR}) {
if (!mesh_size.symmetry(dir)) {
l_region_min[dir - 1] =
GetLLFromMeshCoordinate(dir, lrlev, ref_size.xmin(dir));
GetLegacyLLFromMeshCoordinate(dir, lrlev, ref_size.xmin(dir));
l_region_max[dir - 1] =
GetLLFromMeshCoordinate(dir, lrlev, ref_size.xmax(dir));
GetLegacyLLFromMeshCoordinate(dir, lrlev, ref_size.xmax(dir));
l_region_min[dir - 1] =
std::max(l_region_min[dir - 1], static_cast<std::int64_t>(0));
l_region_max[dir - 1] =
Expand All @@ -335,7 +335,7 @@ Mesh::Mesh(ParameterInput *pin, ApplicationInput *app_in, Packages_t &packages,
auto current_loc =
LogicalLocation(lrlev, l_region_max[0], l_region_max[1], l_region_max[2]);
// Remove last block if it just it's boundary overlaps with the region
if (GetMeshCoordinate(dir, BlockLocation::Left, current_loc) ==
if (GetLegacyMeshCoordinate(dir, BlockLocation::Left, current_loc) ==
ref_size.xmax(dir))
l_region_max[dir - 1]--;
if (l_region_min[dir - 1] % 2 == 1) l_region_min[dir - 1]--;
Expand Down
18 changes: 10 additions & 8 deletions src/mesh/mesh.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ class Mesh {
PostStepUserDiagnosticsInLoop = PostStepUserDiagnosticsInLoopDefault;

int GetRootLevel() const noexcept { return root_level; }
int GetLegacyTreeRootLevel() const noexcept {
return forest.root_level + forest.forest_level;
int GetLegacyTreeRootLevel() const {
return forest.root_level + forest.forest_level.value();
}

int GetMaxLevel() const noexcept { return max_level; }
Expand Down Expand Up @@ -323,17 +323,19 @@ class Mesh {

// Transform from logical location coordinates to uniform mesh coordinates accounting
// for root grid
Real GetMeshCoordinate(CoordinateDirection dir, BlockLocation bloc,
const LogicalLocation &loc) const {
Real GetLegacyMeshCoordinate(CoordinateDirection dir, BlockLocation bloc,
const LogicalLocation &loc) const {
auto xll = loc.LLCoord(dir, bloc);
auto root_fac = static_cast<Real>(1 << root_level) / static_cast<Real>(nrbx[dir - 1]);
auto root_fac = static_cast<Real>(1 << GetLegacyTreeRootLevel()) /
static_cast<Real>(nrbx[dir - 1]);
xll *= root_fac;
return mesh_size.xmin(dir) * (1.0 - xll) + mesh_size.xmax(dir) * xll;
}

std::int64_t GetLLFromMeshCoordinate(CoordinateDirection dir, int level,
Real xmesh) const {
auto root_fac = static_cast<Real>(1 << root_level) / static_cast<Real>(nrbx[dir - 1]);
std::int64_t GetLegacyLLFromMeshCoordinate(CoordinateDirection dir, int level,
Real xmesh) const {
auto root_fac = static_cast<Real>(1 << GetLegacyTreeRootLevel()) /
static_cast<Real>(nrbx[dir - 1]);
auto xLL = (xmesh - mesh_size.xmin(dir)) /
(mesh_size.xmax(dir) - mesh_size.xmin(dir)) / root_fac;
return static_cast<std::int64_t>((1 << std::max(level, 0)) * xLL);
Expand Down
3 changes: 2 additions & 1 deletion tst/regression/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ if (ENABLE_HDF5)
list(APPEND TEST_DIRS sparse_advection)
list(APPEND TEST_PROCS ${NUM_MPI_PROC_TESTING})
list(APPEND TEST_ARGS "--driver ${PROJECT_BINARY_DIR}/example/sparse_advection/sparse_advection-example \
--driver_input ${CMAKE_CURRENT_SOURCE_DIR}/test_suites/sparse_advection/parthinput.sparse_advection")
--driver_input ${CMAKE_CURRENT_SOURCE_DIR}/test_suites/sparse_advection/parthinput.sparse_advection \
--num_steps 3")
list(APPEND EXTRA_TEST_LABELS "")

endif()
Expand Down
47 changes: 47 additions & 0 deletions tst/regression/test_suites/sparse_advection/sparse_advection.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,27 @@ def Prepare(self, parameters, step):
"parthenon/sparse/enable_sparse=false",
]

# Run a test with two trees
if step == 2:
parameters.driver_cmd_line_args = [
"parthenon/mesh/nx2=32",
"parthenon/job/problem_id=sparse_twotree",
]

# Run a test with two trees and a statically refined region
if step == 3:
parameters.driver_cmd_line_args = [
"parthenon/mesh/nx2=32",
"parthenon/time/nlim=50",
"parthenon/job/problem_id=sparse_twotree_static",
"parthenon/mesh/refinement=static",
"parthenon/static_refinement0/x1min=-0.75",
"parthenon/static_refinement0/x1max=-0.5",
"parthenon/static_refinement0/x2min=-0.75",
"parthenon/static_refinement0/x2max=-0.5",
"parthenon/static_refinement0/level=3",
]
lroberts36 marked this conversation as resolved.
Show resolved Hide resolved

return parameters

def Analyse(self, parameters):
Expand Down Expand Up @@ -77,5 +98,31 @@ def Analyse(self, parameters):
tol=1e-12,
check_metadata=False,
)
if delta != 0:
return False
lroberts36 marked this conversation as resolved.
Show resolved Hide resolved

delta = compare(
[
"sparse_twotree.out0.final.phdf",
parameters.parthenon_path
+ "/tst/regression/gold_standard/sparse_twotree.out0.final.phdf",
],
one=True,
tol=1e-12,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason for the "loose" tolerance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I just copied this tolerance from the other checks. Isn't 1.e-12 what we set for most tests? I don't think this test should be bitwise exact between machines.

check_metadata=False,
)
if delta != 0:
return False

delta = compare(
[
"sparse_twotree_static.out0.final.phdf",
parameters.parthenon_path
+ "/tst/regression/gold_standard/sparse_twotree_static.out0.final.phdf",
],
one=True,
tol=1e-12,
check_metadata=False,
)

return delta == 0
Loading