Changes for coupling with ERF using MultiBlock #1297
Conversation
| @@ -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; } | |||
There was a problem hiding this comment.
raw pointers make me nervous. Is there some way we could be using unique_ptr instead?
There was a problem hiding this comment.
Okay thanks for the suggestion.. I'll surely look into this, it's always good to have a more robust code.
| @@ -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; | |||
There was a problem hiding this comment.
are these needed? Like the build breaks without them? I see you do it in a bunch of places.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Sorry, I mean the forward declarations.
| @@ -47,6 +47,20 @@ public: | |||
| void init_tke(const amrex::Geometry& geom, amrex::MultiFab& tke) const; | |||
|
|
|||
| private: | |||
| // ABL-with-bubble | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Good work, @mukul1992 ! Left a bunch of questions mostly. |
|
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. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| nc += fld->num_comp(); | ||
| } | ||
|
|
||
| // FIXME: need to generalize to lev > 0 somehow |
Check notice
Code scanning / CodeQL
FIXME comment
| 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 |
Check notice
Code scanning / CodeQL
FIXME comment
| @@ -758,6 +809,19 @@ | |||
| return; | |||
| } | |||
|
|
|||
| #ifdef ERF_AMR_WIND_MULTIBLOCK | |||
| if (m_out_fmt == "erf-multiblock") { | |||
| // m_read_erf = sim.get_read_erf(); | |||
Check notice
Code scanning / CodeQL
Commented-out code
| nc += fld->num_comp(); | ||
| } | ||
|
|
||
| // FIXME: need to generalize to lev > 0 somehow |
Check notice
Code scanning / CodeQL
FIXME comment Note
| 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 |
Check notice
Code scanning / CodeQL
FIXME comment Note
| @@ -758,6 +809,19 @@ | |||
| return; | |||
| } | |||
|
|
|||
| #ifdef ERF_AMR_WIND_MULTIBLOCK | |||
| if (m_out_fmt == "erf-multiblock") { | |||
| // m_read_erf = sim.get_read_erf(); | |||
Check notice
Code scanning / CodeQL
Commented-out code Note
|
Sounds good! Let us know how we can help get the CI passing for you or any other issues that we can help with. |
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_erffunction 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:
This PR was tested by running: