Changes for coupling with ERF using MultiBlock #1297

mukul1992 · 2024-10-15T23:47:25Z

Summary

These changes allow coupling with ERF via the MultiBlock mechanism. It includes maintaining a pointer to the multiblock container which is defined in the coupling driver. For reading ERF data, it uses the ABL boundary plane mechanism by defining a read_erf function in the driver and passing it to a function pointer in AMR-Wind.

Pull request type

Please check the type of change introduced:

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 (MacOS M3 arch, GCC 13)
the regression tests
- on GPU
- on CPU (MacOS M3 arch, GCC 13)

CMakeLists.txt

marchdf · 2024-10-16T14:18:18Z

amr-wind/CFDSim.H

@@ -58,6 +63,14 @@ public:
    SimTime& time() { return m_time; }
    const SimTime& time() const { return m_time; }

+#ifdef ERF_AW_MULTIBLOCK
+    void set_mbc(MultiBlockContainer* mbc) { m_mbc = mbc; }


raw pointers make me nervous. Is there some way we could be using unique_ptr instead?

Okay thanks for the suggestion.. I'll surely look into this, it's always good to have a more robust code.

CMakeLists.txt

amr-wind/incflo.H

marchdf · 2024-10-16T14:21:11Z

amr-wind/wind_energy/ABLBoundaryPlane.H

@@ -7,6 +7,10 @@
 #include "amr-wind/utilities/ncutils/nc_interface.H"
 #include <AMReX_BndryRegister.H>

+#ifdef ERF_AW_MULTIBLOCK
+#include "amr-wind/wind_energy/ABLReadERFFcn.H"
+class MultiBlockContainer;


are these needed? Like the build breaks without them? I see you do it in a bunch of places.

do you mean the ifdefs? I could remove them from all places where they are not strictly required. My thinking was that maybe you would prefer using the ifdefs to keep out the multiblock stuff as much as possible.

Sorry, I mean the forward declarations.

amr-wind/utilities/index_operations.H

amr-wind/wind_energy/ABLBoundaryPlane.cpp

marchdf · 2024-10-16T14:25:54Z

amr-wind/wind_energy/ABLFieldInit.H

@@ -47,6 +47,20 @@ public:
    void init_tke(const amrex::Geometry& geom, amrex::MultiFab& tke) const;

 private:
+    // ABL-with-bubble


Is the bubble thing something you want to keep? Or was this just a debug thing? If you want to keep it, it might be better to move this to the physics folder/class. Would you be ok with that?

It is a debug thing where Bruce was using temperature in lieu of a scalar. I would like to use the scalar instead so I'll make that change at some point.

amr-wind/wind_energy/ABLReadERFFcn.H

cmake/amr-wind-utils.cmake

marchdf · 2024-10-16T14:27:40Z

Good work, @mukul1992 ! Left a bunch of questions mostly.

mukul1992 · 2024-10-17T18:14:28Z

Thanks for the comments @marchdf @jrood-nrel.

I should have referenced this while creating the PR, but here's the driver repo for the ERF/AMR-Wind coupling. This may clarify the cmake related changes and some of the code changes.

amr-wind/wind_energy/ABLBoundaryPlane.cpp

+            nc += fld->num_comp();
+        }
+
+        // FIXME: need to generalize to lev > 0 somehow


amr-wind/wind_energy/ABLBoundaryPlane.cpp

+        const int lev = 0;
+        for (amrex::OrientationIter oit; oit != nullptr; ++oit) {
+            auto ori = oit();
+            // FIXME: would be safer and less storage to not allocate all of


amr-wind/wind_energy/ABLBoundaryPlane.cpp

@@ -758,6 +809,19 @@
        return;
    }

+#ifdef ERF_AMR_WIND_MULTIBLOCK
+    if (m_out_fmt == "erf-multiblock") {
+        // m_read_erf = sim.get_read_erf();


amr-wind/wind_energy/ABLBoundaryPlane.cpp

+            nc += fld->num_comp();
+        }
+
+        // FIXME: need to generalize to lev > 0 somehow


amr-wind/wind_energy/ABLBoundaryPlane.cpp

+        const int lev = 0;
+        for (amrex::OrientationIter oit; oit != nullptr; ++oit) {
+            auto ori = oit();
+            // FIXME: would be safer and less storage to not allocate all of


amr-wind/wind_energy/ABLBoundaryPlane.cpp

@@ -758,6 +809,19 @@
        return;
    }

+#ifdef ERF_AMR_WIND_MULTIBLOCK
+    if (m_out_fmt == "erf-multiblock") {
+        // m_read_erf = sim.get_read_erf();


marchdf · 2024-10-21T18:26:03Z

Sounds good! Let us know how we can help get the CI passing for you or any other issues that we can help with.

moprak-nrel

Good work! Have a couple of quick comments.

moprak-nrel · 2024-11-08T15:41:16Z

amr-wind/utilities/index_operations.H

+AMREX_FORCE_INLINE int closest_index_lbound(
+    const amrex::Vector<amrex::Real>& vec, const amrex::Real value)
+{
+    amrex::Real one_minus_eps = 1.0 - 1.e-8;


Is there a reason for the hardcoded 1e-8 here? should this be closer to std::numeric_limits<amrex::Real>::epsilon(); ?

I'm wondering why this is necessary in the first place for lower_bound? Wouldn't you always be guaranteed that vec[idx -1] < value <= vec[idx] (for idx > 1, and value <= vec[0] otherwise)?

moprak-nrel · 2024-11-08T16:15:30Z

amr-wind/wind_energy/ABLBoundaryPlane.cpp

 {
    const size_t nc = fld->num_comp();
    const int nstart =
        static_cast<int>(m_components[static_cast<int>(fld->id())]);

-    const int idx = utils::closest_index(times, time);
+    int idx;


Is the intention here to have times[idx] < time <= times[idx+1] for multiblock instead of times[idx] <= time < times[idx+1]?

I'm a bit confused but the 1+eps checks below, are they necessary with lbound?

moprak-nrel · 2024-11-08T16:36:47Z

amr-wind/wind_energy/ABLBoundaryPlane.cpp

@@ -758,6 +780,35 @@ void ABLBoundaryPlane::read_header()
                m_in_data.define_level_data(ori, pbx, nc);
            }
        }
+    } else if (m_out_fmt == "erf-multiblock") {
+
+        m_in_times.push_back(-1.0e13); // create space for storing time at erf


would std::numeric_limits<amrex::Real>::epsilon() work instead of 1e-13?

…ed on

marchdf requested review from jrood-nrel, marchdf, mbkuhn and moprak-nrel October 16, 2024 14:16

marchdf requested changes Oct 16, 2024

View reviewed changes

github-advanced-security bot found potential problems Oct 18, 2024

View reviewed changes

mukul1992 and others added 20 commits November 5, 2024 17:58

initial changes for coupling with ERF

8731f51

add missing change from Bruce's amr-wind

87f1837

clean up and move Evolve_MB to the coupling driver repo

dff1456

Switch to project_source_dir

f16227f

Try turning on multiblock flag

4c8132e

add code for multiblock coupling with ERF

c8b634a

add file that defines the type of the read_erf function pointer

8765393

add temperature bubble initialization for coupling test with ERF

4dedbb5

remove evolve_MB that crept in during rebase

ff89358

add ifdef for evolve_mb declaration

3646e1c

remove unnecessary ifdef

35d120c

change ifdef directive name

0aef38e

add back required ifdef

0d435ba

fix floating point comparison

b9db450

change and to && and add paranetheses

f7397c3

change closest_index to closest_index_ubound to be more explicit

27b3d3b

rename to ReadERFFunction

79bcda9

add back Waves2AMR to command, removed by mistake while merging

15a8a0a

change names to be more explicit

e33c28e

clang-format

f63228d

mukul1992 added 2 commits November 5, 2024 18:01

change to ERF_AMR_WIND_MULTIBLOCK for ifdefs

4ccb4b4

code-spell updates

b8d3e94

mukul1992 force-pushed the splitting_build branch from 9e5f85b to b8d3e94 Compare November 6, 2024 19:01

handle NPH time values

e5c9ce4

moprak-nrel requested changes Nov 8, 2024

View reviewed changes

prepare boundaries during initialization only if init iterations turn…

18a59c8

…ed on

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes for coupling with ERF using MultiBlock #1297

Changes for coupling with ERF using MultiBlock #1297

mukul1992 commented Oct 15, 2024

marchdf Oct 16, 2024

mukul1992 Oct 17, 2024

marchdf Oct 16, 2024

mukul1992 Oct 17, 2024

marchdf Oct 21, 2024

marchdf Oct 16, 2024

mukul1992 Oct 17, 2024

marchdf commented Oct 16, 2024

mukul1992 commented Oct 17, 2024

marchdf commented Oct 21, 2024

moprak-nrel left a comment

moprak-nrel Nov 8, 2024

moprak-nrel Nov 8, 2024

moprak-nrel Nov 8, 2024

Changes for coupling with ERF using MultiBlock #1297

Are you sure you want to change the base?

Changes for coupling with ERF using MultiBlock #1297

Conversation

mukul1992 commented Oct 15, 2024

Summary

Pull request type

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marchdf commented Oct 16, 2024

mukul1992 commented Oct 17, 2024

marchdf commented Oct 21, 2024

moprak-nrel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment