-
Notifications
You must be signed in to change notification settings - Fork 129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Is hydro diffusion doing the right thing in spherical coordinates? #171
Comments
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Can we re-open this issue @felker? Stale-bot closed this on Feb 6, 2019; but @c-white raises some good questions here that were never addressed. I'd raise another one:
Remember that in the explicit integration of diffusive physics, Aside: When producing the STS extension, I assumed that all geometric source terms (when diffusion is enabled) were correct; so if any geometric issues with explicit diffusion exist, they would have also been carried over into STS. My testing of curvilinear STS was limited in #299. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I was looking at the 3 locations in
SphericalPolar::CoordSrcTerms()
where hydro diffusion is accounted for. I don't know much about the diffusion implementation, but it struck me that some of the fluxes being used might not exist in certain cases, and at other times terms might not be accounted for.The r-momentum is apparently affected by theta-fluxes of theta-momentum and phi-fluxes of phi-momentum:
athena/src/coordinates/spherical_polar.cpp
Lines 526 to 529 in 5fd9ea5
But in 1D or 2D will these always be set? Similarly for the theta-momentum:
athena/src/coordinates/spherical_polar.cpp
Lines 554 to 555 in 5fd9ea5
I suppose those arrays could just be 0 if the dimensionality is low enough, so the right thing is always done. I'm more confused by the phi-momentum source term:
athena/src/coordinates/spherical_polar.cpp
Lines 560 to 573 in 5fd9ea5
Since
use_x2_fluxes
istrue
if and only if we are in 2D or 3D, it seems this term only gets added in 1D, in which case the referenced fluxes are 0 (going by the above logic). And it seems like the theta-phi viscous stress is not properly being accounted for in 2D and 3D.Again, I'm not really familiar with how diffusion works in the code, so maybe this is all fine. But I'm working on a project that involves taking a very close look at curvilinear source terms, so I'm trying to understand exactly what's happening in this function.
The text was updated successfully, but these errors were encountered: