Exawind / amr-wind

AMReX-based structured wind solver
https://exawind.github.io/amr-wind
Other
103 stars 78 forks source link

Fix ghost cells being used for mac velocity BC functions #1093

Closed marchdf closed 3 weeks ago

marchdf commented 3 weeks ago

Summary

Fixing the fact that the UDFs are called on cell-centered and face centered velocities and, therefore, the indices of the cells should be set appropriately.

Pull request type

Please check the type of change introduced:

Checklist

Left to do:

The following is included:

This PR was tested by running:

Additional background

With these changes, the fillpatch of the mac velocities now touches the following indices depending on the face-centered mac velocity component. For umac, we need to be touching -1 and 41 (with n_cell = 40). For the others it is -1 and 40.

  MAC_projection                 9         0.01766980429       1.787159811e-14
this face_idir: 0
Rankine.H op: filling 10.73181747 at i=-1
Rankine.H op: filling 10.13957005 at i=41
this face_idir: 1
Rankine.H op: filling 1.016180833 at i=-1
Rankine.H op: filling 0.5357916479 at i=40
this face_idir: 2
Rankine.H op: filling 0 at i=-1
Rankine.H op: filling 0 at i=40

Issue Number: #1076

mbkuhn commented 3 weeks ago

🏆 for this PR name!

marchdf commented 3 weeks ago

This is mostly done. But I am still waiting on something to be cleared up. Basically a bunch of stuff happens in IAMR and PeleLMeX that doesn't happen in incflo and I am not sure which is the right direction for amr-wind. Discussion is on-going in several places and here's one of them: https://github.com/AMReX-Combustion/PeleLMeX/issues/386.

I have that stuff implemented for amr-wind and it causes diffs but no significant changes in the solution. So I can push it up here or ditch it depending on where these discussions go.

marchdf commented 3 weeks ago

If it turns out that stuff in IAMR/PeleLMeX is needed I can add it as a separate PR. Right now it is looking like it is not needed (at least for most cases, maybe needed for variable density?). Discussion is ongoing but shouldn't hold this PR up. I don't expect any diffs with this PR.