Fix oops #1383
Fix oops #1383
Conversation
| @@ -130,10 +129,16 @@ void ERFPC::AdvectWithFlow ( MultiFab* a_umac, | |||
| iv[1] += domain.smallEnd()[1]; | |||
There was a problem hiding this comment.
@AMLattanzi @asalmgren Can lines 124-129 be replaced by ERFParticlesAssignor(p, plo, dxi, domain) (defined in ERFPC.H)? Same with lines 226-231 below?
| @@ -98,8 +98,7 @@ void ERFPC::AdvectWithFlow ( MultiFab* a_umac, | |||
| bool use_terrain = (a_z_height != nullptr); | |||
| auto zheight = use_terrain ? (*a_z_height)[grid].array() : Array4<Real>{}; | |||
There was a problem hiding this comment.
@asalmgren @AMLattanzi how does this make zheight available on the GPU?
There was a problem hiding this comment.
This is due to C++ lambda capture. If a C++ lambda uses [=], it will make a copy of all the variables used in the lambda's function body. These variables are part of the lambda function object. In the case of GPU, the CUDA runtime will copy the lambda function to GPU at kernel launch.
There was a problem hiding this comment.
Thanks, @WeiqunZhang! Makes sense now.
| ParticleReal zlo, zhi; | ||
| if (use_terrain) { | ||
| zlo = Real(0.25) * (zheight(iv[0],iv[1] ,iv[2] ) + zheight(iv[0]+1,iv[1] ,iv[2] ) + | ||
| zheight(iv[0],iv[1]+1,iv[2] ) + zheight(iv[0]+1,iv[1]+1,iv[2] )); | ||
| zhi = Real(0.25) * (zheight(iv[0],iv[1] ,iv[2]+1) + zheight(iv[0]+1,iv[1] ,iv[2]+1) + | ||
| zheight(iv[0],iv[1]+1,iv[2]+1) + zheight(iv[0]+1,iv[1]+1,iv[2]+1)); | ||
| Real lx = (p.pos(0)-plo[0])*dxi[0] - static_cast<Real>(iv[0]-domain.smallEnd()[0]); | ||
| Real ly = (p.pos(1)-plo[1])*dxi[1] - static_cast<Real>(iv[1]-domain.smallEnd()[1]); | ||
| zlo = zheight(iv[0] ,iv[1] ,iv[2] ) * (1.0-lx) * (1.0-ly) + | ||
| zheight(iv[0]+1,iv[1] ,iv[2] ) * lx * (1.0-ly) + | ||
| zheight(iv[0] ,iv[1]+1,iv[2] ) * (1.0-lx) * ly + | ||
| zheight(iv[0]+1,iv[1]+1,iv[2] ) * lx * ly; | ||
| zhi = zheight(iv[0] ,iv[1] ,iv[2]+1) * (1.0-lx) * (1.0-ly) + | ||
| zheight(iv[0]+1,iv[1] ,iv[2]+1) * lx * (1.0-ly) + | ||
| zheight(iv[0] ,iv[1]+1,iv[2]+1) * (1.0-lx) * ly + | ||
| zheight(iv[0]+1,iv[1]+1,iv[2]+1) * lx * ly; | ||
| } else { | ||
| zlo = iv[2] * dx[2]; | ||
| zhi = (iv[2]+1) * dx[2]; |
There was a problem hiding this comment.
@AMLattanzi @asalmgren Can lines 130-145 (or 130-151) be written as a separate function/functor? If functor, it could store zheight and use_terrain as member data? And we could use it for lines 232-252 as well.
There was a problem hiding this comment.
Actually I want to move this functionality into a routine in amrex -- it is used by multiple codes. I was just going to make a new function called something like update_particle_idata that takes one particle and the zheight array. What do you think?
There was a problem hiding this comment.
That would be nice since this is a particle functionality, not an ERF-specific thing. But terrain and zheight are specific to atmospheric flows. To make it more general, could we have xcoord, ycoord, zcoord (defaulted to nullptr?) instead of just zheight that allows mapping in all dimensions? ERF would then call this function with only zheight.
There was a problem hiding this comment.
We're working on the generalized functionality ...
There was a problem hiding this comment.
I think we could parallel what is done in the BinMapper functor. This would move the functionality to AMReX, clean up the ERF code since the above snippet now lives in a functor, and would be similar to what we do without curvilinear coordinates.
|
@asalmgren @AMLattanzi If you agree with my comments above, then I can try to implement them and send a pull request. @gardner48 Tagging you so that you get notified as well. |
No description provided.