From ccc2f250aefbf1ab67093d2366221eef5708b70d Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Thu, 9 May 2024 12:33:05 -0600 Subject: [PATCH 1/5] bugfix --- src/mesh/forest/forest.cpp | 2 +- src/mesh/forest/forest.hpp | 2 +- src/mesh/mesh.cpp | 8 ++++---- src/mesh/mesh.hpp | 19 ++++++++++++------- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/mesh/forest/forest.cpp b/src/mesh/forest/forest.cpp index e321304e073c..65cee50a7fd9 100644 --- a/src/mesh/forest/forest.cpp +++ b/src/mesh/forest/forest.cpp @@ -180,7 +180,7 @@ Forest Forest::HyperRectangular(RegionSize mesh_size, RegionSize block_size, // Sort trees by their logical location in the tree mesh Forest fout; fout.root_level = ref_level; - fout.forest_level = level; + *(fout.forest_level) = level; for (auto &[loc, p] : ll_map) fout.AddTree(p.second); return fout; diff --git a/src/mesh/forest/forest.hpp b/src/mesh/forest/forest.hpp index 579d956c56a1..eec343bc227c 100644 --- a/src/mesh/forest/forest.hpp +++ b/src/mesh/forest/forest.hpp @@ -39,7 +39,7 @@ class Forest { public: int root_level; - int forest_level; + std::optional forest_level{}; void AddTree(const std::shared_ptr &in) { if (trees.count(in->GetId())) { diff --git a/src/mesh/mesh.cpp b/src/mesh/mesh.cpp index 15dc705c06f5..3d6c75cedeb3 100644 --- a/src/mesh/mesh.cpp +++ b/src/mesh/mesh.cpp @@ -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 @@ -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(0)); l_region_max[dir - 1] = @@ -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]--; diff --git a/src/mesh/mesh.hpp b/src/mesh/mesh.hpp index eb85d334b02e..21e93eb3d4c6 100644 --- a/src/mesh/mesh.hpp +++ b/src/mesh/mesh.hpp @@ -192,7 +192,10 @@ class Mesh { int GetRootLevel() const noexcept { return root_level; } int GetLegacyTreeRootLevel() const noexcept { - return forest.root_level + forest.forest_level; + PARTHENON_REQUIRE( + forest.forest_level, + "The level of the forest must be defined to reference the legacy tree."); + return forest.root_level + *(forest.forest_level); } int GetMaxLevel() const noexcept { return max_level; } @@ -323,17 +326,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(1 << root_level) / static_cast(nrbx[dir - 1]); + auto root_fac = static_cast(1 << GetLegacyTreeRootLevel()) / + static_cast(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(1 << root_level) / static_cast(nrbx[dir - 1]); + std::int64_t GetLegacyLLFromMeshCoordinate(CoordinateDirection dir, int level, + Real xmesh) const { + auto root_fac = static_cast(1 << GetLegacyTreeRootLevel()) / + static_cast(nrbx[dir - 1]); auto xLL = (xmesh - mesh_size.xmin(dir)) / (mesh_size.xmax(dir) - mesh_size.xmin(dir)) / root_fac; return static_cast((1 << std::max(level, 0)) * xLL); From 4b9bcda5581390a0ab55b4a1e3b0635f1453af56 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Thu, 9 May 2024 13:29:12 -0600 Subject: [PATCH 2/5] fix new bug --- src/mesh/forest/forest.cpp | 2 +- src/mesh/mesh.hpp | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/mesh/forest/forest.cpp b/src/mesh/forest/forest.cpp index 65cee50a7fd9..e321304e073c 100644 --- a/src/mesh/forest/forest.cpp +++ b/src/mesh/forest/forest.cpp @@ -180,7 +180,7 @@ Forest Forest::HyperRectangular(RegionSize mesh_size, RegionSize block_size, // Sort trees by their logical location in the tree mesh Forest fout; fout.root_level = ref_level; - *(fout.forest_level) = level; + fout.forest_level = level; for (auto &[loc, p] : ll_map) fout.AddTree(p.second); return fout; diff --git a/src/mesh/mesh.hpp b/src/mesh/mesh.hpp index 21e93eb3d4c6..4b4a212be4cc 100644 --- a/src/mesh/mesh.hpp +++ b/src/mesh/mesh.hpp @@ -191,11 +191,8 @@ class Mesh { PostStepUserDiagnosticsInLoop = PostStepUserDiagnosticsInLoopDefault; int GetRootLevel() const noexcept { return root_level; } - int GetLegacyTreeRootLevel() const noexcept { - PARTHENON_REQUIRE( - forest.forest_level, - "The level of the forest must be defined to reference the legacy tree."); - 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; } From 62276a41251600f504a5999a34ef829f6a159cd8 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Thu, 9 May 2024 13:30:21 -0600 Subject: [PATCH 3/5] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b165b68a450b..0bc5e7cf4579 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,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 From 9ba4549d9419f53271460481de8217cb5f22b09a Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Thu, 9 May 2024 14:49:27 -0600 Subject: [PATCH 4/5] Add two more regression tests that have multiple trees --- tst/regression/CMakeLists.txt | 3 +- .../sparse_advection/sparse_advection.py | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/tst/regression/CMakeLists.txt b/tst/regression/CMakeLists.txt index 96a54da6e427..026be61a52e4 100644 --- a/tst/regression/CMakeLists.txt +++ b/tst/regression/CMakeLists.txt @@ -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() diff --git a/tst/regression/test_suites/sparse_advection/sparse_advection.py b/tst/regression/test_suites/sparse_advection/sparse_advection.py index 18c89a444f96..bb321b6da5a4 100644 --- a/tst/regression/test_suites/sparse_advection/sparse_advection.py +++ b/tst/regression/test_suites/sparse_advection/sparse_advection.py @@ -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", + ] + return parameters def Analyse(self, parameters): @@ -77,5 +98,31 @@ def Analyse(self, parameters): tol=1e-12, check_metadata=False, ) + if delta != 0: + return False + + delta = compare( + [ + "sparse_twotree.out0.final.phdf", + parameters.parthenon_path + + "/tst/regression/gold_standard/sparse_twotree.out0.final.phdf", + ], + one=True, + tol=1e-12, + 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 From d285155496835ffdd4b01adf7dd8bc3651c59b01 Mon Sep 17 00:00:00 2001 From: Luke Roberts Date: Wed, 15 May 2024 13:01:37 -0600 Subject: [PATCH 5/5] Add printouts for failed tests --- .../test_suites/sparse_advection/sparse_advection.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tst/regression/test_suites/sparse_advection/sparse_advection.py b/tst/regression/test_suites/sparse_advection/sparse_advection.py index bb321b6da5a4..e4d812da997a 100644 --- a/tst/regression/test_suites/sparse_advection/sparse_advection.py +++ b/tst/regression/test_suites/sparse_advection/sparse_advection.py @@ -99,6 +99,7 @@ def Analyse(self, parameters): check_metadata=False, ) if delta != 0: + print("Sparse advection failed for standard AMR grid setup.") return False delta = compare( @@ -112,6 +113,7 @@ def Analyse(self, parameters): check_metadata=False, ) if delta != 0: + print("Sparse advection failed for two-tree AMR grid setup.") return False delta = compare( @@ -124,5 +126,7 @@ def Analyse(self, parameters): tol=1e-12, check_metadata=False, ) + if delta != 0: + print("Sparse advection failed for two-tree SMR grid setup.") return delta == 0