search

AMReX-Codes / amrex

AMReX: Software Framework for Block Structured AMR
https://amrex-codes.github.io/amrex
Other
503 stars 336 forks source link

Suspected Repeated data in MLCellLinOp #3950

Closed ankithadas closed 1 month ago

ankithadas commented 1 month ago

Dear AMReX team,

I wanted some clarification regarding some data structs in AMReX_MLCellLinOp. I think m_bndry_sol (Vector of MLMGBndry) and m_bcondloc (Vector of BndryCondLoc) store the same data and achieve the same purpose. Also the values generated in m_maskvals (Vector of MultiMask for each face) is present in m_bndry_sol. Lastly, the ordering of boundary conditions (bcond in BndryData) used in MLMGBndry (Outer is face, inner is component) is different from BndryCondLoc (Outer is component, inner is face)

Are these bugs ?

Kind regards, Ankith

WeiqunZhang commented 1 month ago

m_bndry_sol is a Vector with the size of the number of AMR levels. m_bcondloc is a Vector of Vector with the outer Vector for AMR levels and inner Vector for MG levels at each AMR levels. m_bndry_sol is for the inhomogeneous boundary of the original unknowns, whereas m_bcondloc is for the homegeneous boundary of the corrections to the unknowns. m_maskvals is Vector of Vector, like m_bcondloc.

As for the ordering, it's possible the current approach is not the best for performance, but it's very unlikely it's a correctness bug.

ankithadas commented 1 month ago

Could we potentially combine m_bndry_sol and m_bcondloc, that is, make m_bndry_sol a Vector of Vector? I believe they store the same data.

WeiqunZhang commented 1 month ago

m_bndry_sol has data for each cell on the faces of boxes. m_bcondloc's data is for faces not cells. If we combine them, a lot of space will be wasted.

WeiqunZhang commented 1 month ago

I believe they store the same data.

No, they do not.

WeiqunZhang commented 1 month ago

As mentioned above, m_bndry_sol is for unknowns, which have inhomogeneous boundary data, whereas m_bcondloc is for corrections, which has homogeneous bounadry data. Because it's homogeneous (i.e., zero), we do not need to store data at cell level.

ankithadas commented 1 month ago

To clarify, m_bndry_sol stores the physical boundary condition type, physical boundary values (inhomogeneous boundary data), and 1 ghost layer of boundary data for all boxes in BoxArray ? I am assuming the last one is for filling coarse-fine boundaries for AMR. Is that correct ?

WeiqunZhang commented 1 month ago

Assuming data for all boxes in BoxArray means all processes collectively own them, yes.

ankithadas commented 1 month ago

Thank you for the clarification!