search

Exawind / amr-wind

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

Geostrophic Forcing flaws #1026

Closed mbkuhn closed 3 months ago

mbkuhn commented 5 months ago

I see two problems in the Geostrophic Forcing routine

  1. The intended vertical forcing component (in z) has a 1 as the component index, instead of a 2. This is a simple bug to correct. https://github.com/Exawind/amr-wind/blob/main/amr-wind/equation_systems/icns/source_terms/GeostrophicForcing.cpp#L123
  2. Although the possibility of a vertical component is implied by the "is_horizontal" option, it seems like the implementation does not allow for a nonzero vertical component. This seems like a flaw in the coupling with the parameters from Coriolis Forcing. https://github.com/Exawind/amr-wind/blob/main/amr-wind/equation_systems/icns/source_terms/GeostrophicForcing.cpp#L53
mbkuhn commented 5 months ago

@gdeskos @mchurchf I'd love your input on what things are supposed to look like regarding point 2. above.

github-actions[bot] commented 4 months ago

This issue is stale because it has been open 30 days with no activity.

gdeskos commented 4 months ago

@mbkuhn In this line https://github.com/Exawind/amr-wind/blob/7c660c1685cfa85d875acd98b769d14cbf9c0019/amr-wind/equation_systems/icns/source_terms/GeostrophicForcing.cpp#L162 there is a bug. The index should be 2 instead of 1?

mbkuhn commented 4 months ago

@gdeskos that's true, and that is one of the things I noted originally. However, I'm also saying that forcing[2] is always 0.0. So even if this bug is fixed, there's no possibility of there being a src_term component in z. That seems like a feature is missing, given that the hfac factor is there. Is there a formula for what that component should be, if it's turned on?

marchdf commented 3 months ago

Discussed: for now, @mbkuhn is going to fix the one line bug and then close this issue