Add drag-coefficient formulation of the ib wall model for the flat pl… #13
Conversation
tonyinme
left a comment
tonyinme
left a comment
There was a problem hiding this comment.
Looks good!
Can you add some documentation?
amr-wind/immersed_boundary/IB.cpp
Outdated
| { | ||
| m_ib_levelset.set_default_fillpatch_bc(sim.time()); | ||
| m_ib_normal.set_default_fillpatch_bc(sim.time()); | ||
| // nmatula source need to be added here? |
There was a problem hiding this comment.
You need to add the source there (after you declare the field within IB.H)
amr-wind/immersed_boundary/IB.cpp
Outdated
| ib->update_positions(); | ||
| ib->write_outputs(); | ||
| } | ||
| // nmatula any other places where the source term needs to be added? |
psakievich
left a comment
psakievich
left a comment
There was a problem hiding this comment.
I don't see any reason to not merge this, but I've added a few comments on how to make this code more general and modular going forward. I would prefer to see these adopted but I won't hold up merging if it get's us testing faster
| IB.IB1.type = Flat | ||
| IB.IB1.height = 0.1 | ||
| IB.IB1.drag_coefficient = 0.1 | ||
| IB.IB1.eps_spread = 0.3 |
There was a problem hiding this comment.
I would think it would be good to have all our complex terrain parameters under the ParmParser IB.ComplexTerrain. This would segregate cases where we've implemented the model vs the ones we haven't (cylinder, etc). @gdeskos what are your thoughts? it looks like the parser isn't setup that way.
There was a problem hiding this comment.
@psakievich this is an idea I have been battling with for some time. The idea behind the label is that you are creating an object (e.g. IB1) and then you assign a type which in this case is the Flat terrain, so that you can add more than one IBs (for example one hills and one box) and then compute forces and everything else separately. Also this allows you to specialize and read input according to the type. If we do IB.ComplexTerrain then you can't add different IB objects
There was a problem hiding this comment.
It seems like we should be able to do this with a few minor changes. The actuator stuff works in a way where you can assign the same parameter at different scopes. In terms of reading the input you can still make specialized inputs, you just need to partition the parsing into a general function and a specialized function. One potential syntax would be IB.ComplexTerrain.CT1 for a hill and IB.IB1 for a cylinder. You could then setup the parser to set parameters at the IB.ComplexTerrain scope that would become the default for all the complex terrain objets unless you overrides them.
Let's not worry about it for now though. This is not really a critical feature. It is just slightly more work on the user for now. We can perfect the interface after we prove the method.
| const amrex::Real u = varr(i, j, k, 0); | ||
| const amrex::Real v = varr(i, j, k, 1); | ||
| const amrex::Real w = varr(i, j, k, 2); | ||
| const amrex::Real vmag = std::sqrt(u * u + v * v + w * w); |
There was a problem hiding this comment.
This looks fine. However if you want to reduce the syntax a lot of these tensor and vector operations are predefined for the vs::Vector and vs::Tensor` types through c++operators. We use them a lot in the actuator code and it makes writing the math less prone to error. example.
| }; | ||
|
|
||
| template <> | ||
| struct ComputeSourceOp<Flat> |
There was a problem hiding this comment.
We can write this in a general form so it is applied to all the complex terrain ops and doesn't have to be re-written every time through templates. That would look like this:
template <typename IbType, std::enable_if_t<std::is_base_of<ComplexTerrainType, IbType>::value>>You would need to move this to IBOps.H and adjust the data types to be based off your template type, but it would make it so we only had to write this function once.
A couple of examples of this can be found in the ActuatorDisk PR I'm working on now: Exawind#583
Zero tau where wall model is active
Fix bugs and do some more dev
4 participants
…ate case.