From ec1f94f45ddfc0fe9cc9d83e8e4058d9b48af1b7 Mon Sep 17 00:00:00 2001 From: Ben Dudson Date: Wed, 13 Aug 2025 10:29:05 -0700 Subject: [PATCH 1/2] snes: Print a warning if the coloring is non-symmetric. Added comments to explain why the coloring may be non-symmetric around an X-point. --- src/solver/impls/snes/snes.cxx | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/solver/impls/snes/snes.cxx b/src/solver/impls/snes/snes.cxx index 72891941b6..134b63c2f8 100644 --- a/src/solver/impls/snes/snes.cxx +++ b/src/solver/impls/snes/snes.cxx @@ -377,8 +377,8 @@ int SNESSolver::init() { auto n_cross = (*options)["stencil:cross"] .doc("Extent of stencil (cross)") .withDefault(0); - //Set n_taxi 2 if nothing else is set - //Probably a better way to do this + // Set n_taxi 2 if nothing else is set + // Probably a better way to do this if (n_square == 0 && n_taxi == 0 && n_cross == 0) { output_info.write("Setting solver:stencil:taxi = 2\n"); n_taxi = 2; @@ -485,7 +485,7 @@ int SNESSolver::init() { d_nnz.reserve(nlocal); for (int i = 0; i < nlocal; ++i) { - //Assume all elements in the z direction are potentially coupled + // Assume all elements in the z direction are potentially coupled d_nnz.emplace_back(d_nnz_map3d[i].size() * mesh->LocalNz + d_nnz_map2d[i].size()); o_nnz.emplace_back(o_nnz_map3d[i].size() * mesh->LocalNz @@ -598,9 +598,21 @@ int SNESSolver::init() { MatAssemblyBegin(Jfd, MAT_FINAL_ASSEMBLY); MatAssemblyEnd(Jfd, MAT_FINAL_ASSEMBLY); - //The above will probably miss some non-zero entries at process boundaries - //Making sure the colouring matrix is symmetric will in some/all(?) - //of the missing non-zeros + { + // Test if the matrix is symmetric + // Values are 0 or 1 so tolerance (1e-5) shouldn't matter + PetscBool symmetric; + ierr = MatIsSymmetric(Jfd, 1e-5, &symmetric); CHKERRQ(ierr); + if (!symmetric) { + output_warn.write("Jacobian pattern is not symmetric\n"); + } + } + + // The above can miss entries around the X-point branch cut: + // The diagonal terms are complicated because moving in X then Y + // is different from moving in Y then X at the X-point. + // Making sure the colouring matrix is symmetric does not + // necessarily give the correct stencil but may help. if ((*options)["force_symmetric_coloring"] .doc("Modifies coloring matrix to force it to be symmetric") .withDefault(false)) { From cf7e0437af135d4812ae77bf9819dbb918c7a838 Mon Sep 17 00:00:00 2001 From: bendudson <219233+bendudson@users.noreply.github.com> Date: Wed, 13 Aug 2025 22:37:55 +0000 Subject: [PATCH 2/2] Apply clang-format changes --- src/solver/impls/snes/snes.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/solver/impls/snes/snes.cxx b/src/solver/impls/snes/snes.cxx index 134b63c2f8..2bb163f324 100644 --- a/src/solver/impls/snes/snes.cxx +++ b/src/solver/impls/snes/snes.cxx @@ -602,7 +602,8 @@ int SNESSolver::init() { // Test if the matrix is symmetric // Values are 0 or 1 so tolerance (1e-5) shouldn't matter PetscBool symmetric; - ierr = MatIsSymmetric(Jfd, 1e-5, &symmetric); CHKERRQ(ierr); + ierr = MatIsSymmetric(Jfd, 1e-5, &symmetric); + CHKERRQ(ierr); if (!symmetric) { output_warn.write("Jacobian pattern is not symmetric\n"); }