AMReX-Astro / AMReX-Astro/Castro

`Radiation::filBndry` periodic path only fills one component, breaking multigroup boundary data

Open Beginner friendly
#3,210 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

Location

Source/radiation/Radiation.cpp:1384, Source/radiation/Radiation.cpp:1391, Source/radiation/Radiation.cpp:1400, Source/radiation/Radiation.cpp:1408

Problem

In Radiation::filBndry, the non-periodic branch correctly handles Radiation::nGroups components, but the periodic branch allocates temporary MultiFabs with only 1 component and copies/combines only 1 component.

This is inconsistent with callers like getBndryDataMG() that pass a multigroup BndryRegister.

Impact
  • For periodic domains with multigroup radiation (nGroups > 1), only group 0 coarse boundary data is filled.
  • Remaining groups can stay stale/sentinel values, producing incorrect multigroup boundary conditions.
Steps to Reproduce
  1. Run a multigroup (nGroups > 1) case with periodic boundaries that exercises coarse-fine radiation boundary fills.
  2. Inspect coarse boundary register values after filBndry().
  3. Observe only one group being populated.
Expected Behavior

Periodic and non-periodic branches should both fill all Radiation::nGroups components.

Actual Behavior

Periodic branch fills only one component.

Proposed Fix

Use Radiation::nGroups (or bdry.nComp()) consistently in periodic branch allocation/copy/linComb.

int ncomp = Radiation::nGroups;

if (need_old_data) {
    sold_tmp.define(grids, dmap, ncomp, n_grow);
    sold_tmp.setVal(0.0);
    MultiFab::Copy(sold_tmp, S_old, Rad, 0, ncomp, 0);
    sold_tmp.FillBoundary(geom.periodicity());
}

if (need_new_data) {
    snew_tmp.define(grids, dmap, ncomp, n_grow);
    snew_tmp.setVal(0.0);
    MultiFab::Copy(snew_tmp, S_new, Rad, 0, ncomp, 0);
    snew_tmp.FillBoundary(geom.periodicity());
}

if (time > new_time - eps) {
    bdry.copyFrom(snew_tmp, n_ghost, 0, 0, ncomp);
} else if (time < old_time + eps) {
    bdry.copyFrom(sold_tmp, n_ghost, 0, 0, ncomp);
} else {
    bdry.linComb(a, sold_tmp, 0, b, snew_tmp, 0, 0, ncomp, n_ghost);
}

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/radiation/Radiation.cpp at the four filBndry locations listed in the issue, comparing the periodic and non-periodic branches. Trace the multigroup BndryRegister path through getBndryDataMG(). Done means periodic coarse-fine fills populate all Radiation::nGroups components, including interpolation between old and new data.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.