Boundary conditions by igirault · Pull Request #1 · igirault/PHARE

igirault · 2025-12-10T14:29:49Z

Start implementing boundary conditions

post rebase

Squashed commits WIP scalar -> scalar or tensor Add documentation Remove attributes relative to boundary location and physical quantity in field boundary condition definition. fix some merging issues WIP move some definitions, cosmetic changes WIP

PhilipDeegan · 2026-02-24T17:10:56Z

src/amr/data/field/refine/field_refine_patch_strategy.hpp

+#include "SAMRAI/geom/CartesianPatchGeometry.h"
+#include "SAMRAI/hier/PatchGeometry.h"


typically SAMRAI headers should come after our amr headers

we want to find mpi before samrai does so we can ignore certain warnings and override somethings (see: src/core/def/phare_mpi.hpp)

this file is a good example

so we can spot duplicates the group ordering should be from shortest line to longest and lines of the same length organised alphabetically from first differing character

Yeah I have not been very rigorous and clean on header includes up to now. I agree they must be sorted in some way, but sorting by length has the disadvantage of breaking logical grouping like here

Couldn't we switch to alphabetical order within groups of headers, with groups ordering ( 1 PHARE namespaces, 2 SAMRAI, 3 STL) and enforce everything via clang-format ?

we don't let clang-format sort includes

PhilipDeegan · 2026-02-24T17:15:15Z

src/amr/data/field/refine/field_refine_patch_strategy.hpp

+
+namespace PHARE::amr
+{
+using core::dirX;


I don't think you're using this, but also try to avoid using statements at namespace scope, there can be unintended side affects

PhilipDeegan · 2026-02-24T17:16:26Z

src/amr/data/field/field_data_traits.hpp

+namespace PHARE
+{
+namespace amr
+{


new files namespacing should use namespace PHARE::amr to minimize indentation

PhilipDeegan · 2026-02-24T17:18:07Z

src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp

 #include "amr/data/field/refine/field_moments_refiner.hpp"
 #include "amr/data/field/refine/field_refine_operator.hpp"
 #include "amr/data/field/refine/electric_field_refiner.hpp"
+#include "amr/data/field/refine/magnetic_field_init_refiner.hpp"


not sure if this was you, but it is a duplicate you will spot if you consider the organising I suggested

PhilipDeegan · 2026-02-24T17:19:36Z

src/amr/messengers/mhd_messenger.hpp

+            EalgoPatchGhost.registerRefine(*e_id, *e_id, *e_id, EfieldRefineOp_,
+                                           nonOverwriteInteriorTFfillPattern);


you should check if @UCaromel has these changes

PhilipDeegan · 2026-02-24T17:28:59Z

src/core/boundary/boundary_defs.hpp

+
+#include "core/data/grid/gridlayoutdefs.hpp"
+
+#include "unordered_map"


system includes use <>

PhilipDeegan · 2026-02-24T17:32:59Z

src/core/boundary/boundary_defs.hpp

+    auto it = typeMap_.find(name);
+    if (it == typeMap_.end())
+        throw std::runtime_error("Wrong boundary location name = " + name);
+    return it->second;


if (auto it = typeMap_.find(name); it != typeMap_.end()) return it->second; throw std::runtime_error("Wrong boundary location name = " + name);

or it could be

if (typeMap_.count(name)) return typeMap_.at(name); throw std::runtime_error("Wrong boundary location name = " + name);

iterators are not deprecated, but kinda verbose

PhilipDeegan · 2026-02-24T17:36:51Z

src/core/data/field/field_traits.hpp

+    { field.name() } -> std::convertible_to<std::string const&>;
+    { field.physicalQuantity() } -> std::same_as<typename T::physical_quantity_type const&>;
+
+    { field.isUsable() } -> std::same_as<bool>;


I don't think this should be here, this is more about samrai and how we set buffers
but a field should just be about data/size/shape/etc tbh

PhilipDeegan · 2026-02-24T17:44:04Z

src/core/numerics/boundary_condition/field_antisymmetric_boundary_condition.hpp

+                           Box<std::uint32_t, dimension> const& localGhostBox,
+                           GridLayoutT const& gridLayout, double const time)
+    {
+        constexpr std::array<QtyCentering, N> centerings = {Centerings...};


99% sure you don't need the <QtyCentering, N> whether you do remove it I'll leave to you

I need it here to pass the direction as template parameter to apply_specialized

what I meant specifically, initially, is the array could also look like

constexpr static std::array centerings{Centerings...}; the templates can be inferred.

PhilipDeegan · 2026-02-24T17:45:42Z

src/core/numerics/boundary_condition/field_boundary_condition.hpp

+     * @brief Define comparison of field boundary conditions based on the enum @c
+     * ScalarOrTensorFieldT.
+     */
+    std::strong_ordering operator<=>(This const& other) const


Yeah this will disappear as it is actually not necessary

it would have been our first operator of this kind, I hadn't really understood what it was for just yet

It defines all comparison operators right away without manually defining <,>,>=,<=,==,!=

yeah I know it as the "spaceship" operator, I meant your usage in the current context ;)

I see sorry it was to define priorities between boundary conditions at corner and edges

PhilipDeegan · 2026-02-24T17:58:53Z

src/core/utilities/point/point.hpp


+
+        template<auto direction, auto offset>
+        NO_DISCARD constexpr auto neighbor() const


not sure I like this, something to discuss with nico maybe

I would suggest something like but for a point, not sure I see the benefit to the template params

used here for instance. Isn't it useful (more efficient) in a context where the offset and the direction are known at compile time ?

result[d] += static_cast<Type>(offset);

I think at this point, there is implicit runtime copies, so it's kinda just delaying the inevitable.

PhilipDeegan · 2026-02-24T18:00:37Z

tests/amr/data/tensorfield/tensor_field_data/CMakeLists.txt

+cmake_minimum_required (VERSION 3.20.1)
+project(test-tensor-field-data)
+
+set(SOURCES_INC


if you end up with more than one test file you might want something more like this

PhilipDeegan · 2026-02-25T09:57:39Z

src/amr/data/field/refine/field_refine_patch_strategy.hpp

+
+
+protected:
+    /// Reference to the resources manager.


seems a bit redundant

UCaromel · 2026-02-25T10:23:35Z

src/amr/data/field/refine/magnetic_refine_patch_strategy.hpp

 {
 using core::dirX;
 using core::dirY;
 using core::dirZ;


could probably be moved at private class scope as @PhilipDeegan suggested

UCaromel · 2026-02-25T10:26:51Z

src/core/data/grid/gridlayout_traits.hpp

+ *
+ * @see GridLayout
+ *
+ */


Could we want more minimal concepts to make maintenance easier ? Not sure if LSP can understand less explicit concepts tho.

Sure there are choices to be made about what should be in the concept or not. But if a definition is missing, code editors like VSCode will not provide hints, auto-completion, definition overview, etc...

something like this could be simpler than manually listing all the functions expected, assuming it works for LSP

PhilipDeegan · 2026-02-25T10:29:09Z

src/core/numerics/boundary_condition/field_antisymmetric_boundary_condition.hpp

+        }();
+
+        for_N<N>([&](auto i) {
+            constexpr QtyCentering centering = centerings[i];


and this can be auto if you like, I think it's clear what centerings are

PhilipDeegan · 2026-02-26T18:11:54Z

src/core/utilities/box/box.hpp

        }
    }

+    NO_DISCARD auto rbegin() { return reverse_iterator{this, upper}; }


we're trying to minimize the use of the box iterator for performance reasons

we should talk about why you need this to see if we can not use it

This is because for some boundary conditions (cf FieldDivergenceFreeTransverseNeumanBoundaryCondition) I need to fill ghost cells in a particular order.

I actually let Gemini write this reverse iterator by showing it the direct one to spare my time. I just wanted something that works for now, regardless of performance.

Don't you agree that we should be able to write an iterator that is efficient ?

I've demonstrated that the new version is more efficient, in some contexts at least, it should work for reverse iteration, and hopefully never need std::uint{-1} by using something like this

PhilipDeegan · 2026-02-26T18:13:59Z

src/core/utilities/box/box.hpp

+    {
+        // Point precisely to the index that index_[0]-- creates after passing 'lower'
+        auto end_idx = lower;
+        end_idx[0]--;


end_idx[0]--; has an implicit copy which I'm not positive can be optimized out

so you should aim for --end_idx[0]; when possible (like here)

PhilipDeegan · 2026-02-26T18:19:09Z

src/core/utilities/box/box.hpp

+            using signed_t = std::make_signed_t<Type>;
+            if (static_cast<signed_t>(index_[0]) >= static_cast<signed_t>(box_->lower[0]))


you're going to need to explain this to me, I don't see how whether it's signed or unsigned should matter

is for Box<std::uint32_t. dim>::lower[0] == 0 ?

Yes exactly

igirault force-pushed the boundary_conditions branch 3 times, most recently from 6dddb01 to ae3e40c Compare December 11, 2025 15:02

igirault force-pushed the boundary_conditions branch 4 times, most recently from 7128979 to 34d516c Compare January 26, 2026 10:20

igirault changed the base branch from master to mhd January 26, 2026 18:41

igirault force-pushed the boundary_conditions branch 5 times, most recently from 54e693d to c00493e Compare January 28, 2026 15:38

igirault force-pushed the mhd branch from 6441d94 to 10e56fc Compare February 3, 2026 09:39

igirault force-pushed the boundary_conditions branch 10 times, most recently from 725bf56 to 4024d87 Compare February 5, 2026 22:06

igirault force-pushed the mhd branch from 10e56fc to 7136054 Compare February 9, 2026 07:14

igirault force-pushed the boundary_conditions branch 5 times, most recently from f8780d7 to 5945639 Compare February 10, 2026 15:21

hlld

a30d74e

post rebase

igirault force-pushed the mhd branch from 7136054 to 5dce421 Compare February 19, 2026 12:00

igirault force-pushed the boundary_conditions branch from 4ee589b to 3378ce7 Compare February 19, 2026 12:22

WIP

99c4652

Squashed commits WIP scalar -> scalar or tensor Add documentation Remove attributes relative to boundary location and physical quantity in field boundary condition definition. fix some merging issues WIP move some definitions, cosmetic changes WIP

igirault force-pushed the boundary_conditions branch from 3378ce7 to 99c4652 Compare February 19, 2026 12:48

fix parsing of boundary conditions; correct box_reverse_iterator

fab001f

igirault force-pushed the boundary_conditions branch 2 times, most recently from 08288b4 to 920dcb1 Compare February 24, 2026 10:22

fix rebase errors

db16aa1

igirault force-pushed the boundary_conditions branch from 920dcb1 to db16aa1 Compare February 24, 2026 12:04

PhilipDeegan reviewed Feb 24, 2026

View reviewed changes

PhilipDeegan reviewed Feb 25, 2026

View reviewed changes

UCaromel reviewed Feb 25, 2026

View reviewed changes

PhilipDeegan reviewed Feb 25, 2026

View reviewed changes

igirault added 2 commits February 25, 2026 12:11

phil's remarks

951ddcc

get reflective boundary to work; handle edges and corners;

c06ca6f

PhilipDeegan reviewed Feb 26, 2026

View reviewed changes

		#include "SAMRAI/geom/CartesianPatchGeometry.h"
		#include "SAMRAI/hier/PatchGeometry.h"

		EalgoPatchGhost.registerRefine(e_id, e_id, *e_id, EfieldRefineOp_,
		nonOverwriteInteriorTFfillPattern);


		#include "core/data/grid/gridlayoutdefs.hpp"

		#include "unordered_map"



		template<auto direction, auto offset>
		NO_DISCARD constexpr auto neighbor() const

		using signed_t = std::make_signed_t<Type>;
		if (static_cast<signed_t>(index_[0]) >= static_cast<signed_t>(box_->lower[0]))

Conversation

igirault commented Dec 10, 2025

Uh oh!

PhilipDeegan Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilipDeegan Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilipDeegan Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilipDeegan Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilipDeegan Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igirault Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igirault Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

PhilipDeegan Feb 24, 2026 •

edited

Loading

PhilipDeegan Feb 24, 2026 •

edited

Loading

PhilipDeegan Feb 24, 2026 •

edited

Loading

PhilipDeegan Feb 24, 2026 •

edited

Loading

PhilipDeegan Feb 24, 2026 •

edited

Loading

igirault Feb 25, 2026 •

edited

Loading

igirault Feb 25, 2026 •

edited

Loading

PhilipDeegan Feb 26, 2026 •

edited

Loading

PhilipDeegan Feb 26, 2026 •

edited

Loading