GRTLCollaboration / GRTeclyn

Port of GRChombo to AMReX - under development!
BSD 3-Clause "New" or "Revised" License
4 stars 2 forks source link

Refactor/fix boundary condition code #62

Open mirenradia opened 3 months ago

mirenradia commented 3 months ago

The boundary condition code is in a bit of a mess. I haven't tested anything beyond Sommerfeld and reflective conditions but I think these may be the only working non-periodic ones.

In the Sommerfeld case, we now apply the condition within the valid domain unlike for GRChombo (where we apply it in the ghost cells exterior to the valid domain). AMReX can handle extrapolating conditions but this is then applied in the exterior ghost cells (we actually do this currently in the Sommerfeld case to avoid NaNs not that it should affect anything). Is this OK or, for consistency, if using mixed BCs, do we need to apply the extrapolation in the same cells as for Sommerfeld? What do you think @KAClough?

The existing code should also be refactored/renamed to make it clear these are separate and in addition to AMReX's boundary condition code.

KAClough commented 3 months ago

Yes this is a good point. For the mixed BCs it would need to be consistent. I think it would be nice to have the option to use the AMReX BCs but also I do like being able to have the framework to create our own bespoke ones. So I would say for ours we should consistently apply them within the domain.

mirenradia commented 3 months ago

The problem is that AMReX BCs can't be applied to the RHS which we need to do for static and Sommerfeld BCs.