AMReX-Astro / AMReX-Astro/Castro

`hse_fill` ignores HSE Newton non-convergence on GPU for implemented boundaries

Open
#3,252 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

ai-code-audit BCs bug :bug: GPU
Dominant language
C++
Stars
340
Forks
105
Avg merge
3d 8h
Merged PRs (30d)
8

Description

Summary

In all implemented HSE boundary integrations (-X, +X, -Y, +Y, -Z), the !converged_hse failure path is compiled only for non-GPU builds. On GPU, the code silently proceeds after exhausting iterations, using unconverged density/pressure states.

Location

  • Source/problems/hse_fill.cpp:154
  • Source/problems/hse_fill.cpp:345
  • Source/problems/hse_fill.cpp:543
  • Source/problems/hse_fill.cpp:735
  • Source/problems/hse_fill.cpp:932

Problem Details

Each boundary block follows this pattern:

[[maybe_unused]] bool converged_hse = false;
for (int iter = 0; iter < hse::MAX_ITER; iter++) { ... }

#ifndef AMREX_USE_GPU
if (!converged_hse) {
    amrex::Error("... failure to converge ...");
}
#endif

So in GPU builds, convergence failure is neither reported nor handled, and fill continues with the last Newton iterate.

Impact

  • Silent invalid ghost-state fills when Newton solve fails.
  • Hard-to-diagnose boundary-driven instabilities on GPU runs.
  • CPU/GPU behavior divergence for identical inputs.

Suggested Patch

Use a device-visible failure flag and check it on host after each kernel, instead of CPU-only error paths.

Example pattern:

diff --git a/Source/problems/hse_fill.cpp b/Source/problems/hse_fill.cpp
--- a/Source/problems/hse_fill.cpp
+++ b/Source/problems/hse_fill.cpp
@@
-            amrex::ParallelFor(gbx,
+            int hse_fail = 0;
+            amrex::ParallelFor(gbx,
             [=] AMREX_GPU_DEVICE (int i, int j, int k) noexcept
             {   
                 ...
                 if (!converged_hse) {
-#ifndef AMREX_USE_GPU
-                    amrex::Error("ERROR in bc_ext_fill_nd: failure to converge in -X BC");
-#endif
+                    amrex::Gpu::Atomic::Exch(&hse_fail, 1);
                 }
                 ...
             });
+            amrex::Gpu::streamSynchronize();
+            if (hse_fail != 0) {
+                amrex::Error("ERROR in bc_ext_fill_nd: failure to converge in -X BC");
+            }

Apply the same pattern to the other implemented HSE faces.

Prepared by Codex

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in Source/problems/hse_fill.cpp at lines 154, 345, 543, 735, and 932, and compare the convergence-failure paths for the five implemented HSE faces. Trace how each boundary is launched on GPU and how host-side errors are reported. Done means GPU builds no longer silently continue after Newton non-convergence and CPU/GPU behavior is consistent for all listed boundaries.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
hpc
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.