One way of fixing UDF fillpatches for face quantities (mac velocities) #1084

mbkuhn · 2024-05-30T17:35:15Z

Summary

Using template arguments to modify the boundary index checks for face quantities. Ended up needing a decent amount of new function / type declarations, though, and I'm not sure how best to use the loop within fillpatch_sibling_fields now that each physbc has a different type.

Pull request type

Checklist

The following is included:

new unit-test(s)
new regression test(s)
documentation for new capability

This PR was tested by running:

the unit tests
- on GPU
- on CPU
the regression tests
- on GPU
- on CPU

Additional background

This may not be the best way to do it... it may make things harder when we try to improve the UDFs. Obviously, this is starting off sloppy and will need a thorough pass through and probably unit tests added as well.

Issue Number: #1076

…s later

mbkuhn · 2024-05-30T17:37:03Z

Running the rankine test with this branch give the following output (relevant parts only)

// Fill mac velocities using velocity BCs
sibling fields FPSL dir i = 0 (umac)
Rankine.H op: filling 10.73181747 at i=-1
Rankine.H op: filling 10.13957005 at i=41
sibling fields FPSL dir i = 1 (vmac)
Rankine.H op: filling 1.016180833 at i=-1
Rankine.H op: filling 0.5357916479 at i=40
sibling fields FPSL dir i = 2 (wmac)
Rankine.H op: filling 0 at i=-1
Rankine.H op: filling 0 at i=40

marchdf · 2024-05-31T13:28:18Z

amr-wind/core/FieldBCOps.H

 namespace amr_wind {

+namespace {
+int CC_idx(int /* idir */)


I wonder if for some of these we can use stuff in: https://github.com/Exawind/amr-wind/blob/main/amr-wind/utilities/DirectionSelector.H#L56. Kinda like we do for IndexSelector

took some inspiration from there.

marchdf · 2024-05-31T13:30:26Z

amr-wind/core/FieldBCOps.H

    constexpr explicit FieldBCNoOp(const Field& /*unused*/) {}

    FunctorType operator()() const { return *this; }
+    FunctorXType opfx() const { return *this; }


Could we template this so we don't have repeats? template <T> T opf() ?

marchdf · 2024-05-31T13:34:52Z

amr-wind/core/FieldBCOps.H

                }

                // Check if the point in question is on the boundary
+                int mod_bigEnd = domain_box.bigEnd(idir) + IdxOp(idir);


const and maybe a more descriptive name? bigEnd_depending_on_if_you_are_cc_or_fc ;-) obv not that either.

marchdf · 2024-05-31T13:36:16Z

amr-wind/core/FieldBCOps.H

    explicit FieldBCDirichlet(const Field& fld) : m_field(fld) {}

    inline FunctorType operator()() const { return FunctorType(m_field); }
+    inline FunctorXType opfx() const { return FunctorXType(m_field); }


I gotta think there's a way to template these... template <typename T> T opf() const {return T(m_field);}. Can you see if that's possible?

I didn't include FunctorType in the templating but that is definitely possible too.

marchdf · 2024-05-31T13:38:21Z

amr-wind/core/FieldBCOps.H

    using InflowOpType = typename InflowOp::DeviceType;
    using WallOpType = typename WallOp::DeviceType;
-    using FunctorType = DirichletOp<InflowOpType, WallOpType>;
+    using FunctorType = DirichletOp<InflowOpType, WallOpType, &CC_idx>;


I think we don't need CC_idx I think we can leave it empty and use a default.

marchdf · 2024-05-31T13:38:51Z

amr-wind/core/FieldBCOps.H

            m_wall_op.device_instance()};
    }

+    inline FunctorXType opfx() const


with the templating I think we consolidate all these.

marchdf · 2024-05-31T13:41:10Z

amr-wind/core/FieldFillPatchOps.H

-            amrex::PhysBCFunct<amrex::GpuBndryFuncFab<Functor>> physbc(
-                m_mesh.Geom(lev), bcrec, bc_functor());
+            amrex::PhysBCFunct<amrex::GpuBndryFuncFab<FunctorX>> physbcx(
+                m_mesh.Geom(lev), bcrec, bc_functor_x());


What would be neat here is to keep the for loop and instead do bc_functor_f(i). Or maybe it becomes easier with the templating.

I'm having a hard time getting the loop to work. The problem is that the type of physbcx depends on the direction, in addition to the bc_functor type. I have templatized the FunctorF type, but the template argument has to be a const expression, so it cannot be according to the index i. Making bc_functor output different things depending on an input argument is a problem, too, because each output would be a different type. I also can't put the physbcs into a vector because they're different types. Not sure if there is a way forward here.

ah. yeah I see the issue. I need to think about this....

marchdf · 2024-05-31T13:46:42Z

Nice work! I left a bunch of comments/thoughts/things to try out. LMK if you want help with these.

I think this mostly takes care of the first bit: getting the right indices to be used in the UDF op. The comments I had in the original issue were more focused on defining a different op for the faces. I am not sure we want to go down that road but it's the "right thing to do" eventually. Though again, getting those precise details right is probably only important for BDS and the mac vel in the ghost cells.

…ore merging

amr-wind/core/FieldFillPatchOps.H

+            /*
            for (int i = 0; i < static_cast<int>(mfabs.size()); i++) {
                amrex::FillPatchSingleLevel(
                    *mfabs[i], nghost, time, {ffabs[i]}, {time}, 0, 0, 1,
                    m_mesh.Geom(lev), physbc, i);
            }
+            */


marchdf · 2024-06-11T15:39:07Z

This is done in #1093

mbkuhn added 2 commits May 30, 2024 11:28

use templates to pass functions to operator, specify type based on face

07f88df

changes for running the case we've been using for testing. revert thi…

977d3e5

…s later

mbkuhn requested a review from marchdf May 30, 2024 17:35

marchdf requested changes May 31, 2024

View reviewed changes

mbkuhn added 4 commits May 31, 2024 13:40

templates!

e197237

rename mod_bigEnd

11a1af2

const

74102ba

removing some things for the sake of CI -- more should be removed bef…

3387d17

…ore merging

github-advanced-security bot found potential problems May 31, 2024

View reviewed changes

marchdf closed this Jun 11, 2024

One way of fixing UDF fillpatches for face quantities (mac velocities) #1084

One way of fixing UDF fillpatches for face quantities (mac velocities) #1084

Uh oh!

Conversation

mbkuhn commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Pull request type

Checklist

Additional background

Uh oh!

mbkuhn commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

marchdf commented May 31, 2024

Uh oh!

Check notice

marchdf commented Jun 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mbkuhn commented May 30, 2024 •

edited

Loading

mbkuhn commented May 30, 2024 •

edited

Loading