Add source term in hydro solver#358
Add source term in hydro solver#358ChunYen-Chen wants to merge 30 commits intogamer-project:mainfrom
Conversation
technic960183
left a comment
technic960183
left a comment
There was a problem hiding this comment.
Only one doc related issue and compile time warning because the interface is not used by any module for now.
Tested Combination
--flu_scheme=MHM_RP --mhd=false --gpu=false
--flu_scheme=MHM --mhd=false --gpu=false
--flu_scheme=MHM_RP --mhd=false --gpu=true
--flu_scheme=MHM --mhd=false --gpu=true
(Produce compile time warnings. e.g., Model_Hydro/GPU_Hydro/CUFLU_Shared_AddSourceTerm.cu(56): warning #177-D: variable "TDir1" was declared but never referenced
--flu_scheme=MHM_RP --mhd=true --gpu=false
--flu_scheme=MHM --mhd=true --gpu=false
--flu_scheme=MHM_RP --mhd=true --gpu=true
(Produce compile time warnings declared but never referenced)
--flu_scheme=MHM --mhd=true --gpu=true
(Produce compile time warnings declared but never referenced)
There was a problem hiding this comment.
[BUG] @ChunYen-Chen Do I understand correctly that Hydro_ConFC2PriCC_MHM() must be called even when disabling MHD in order to set g_PriVar_Half[] for Hydro_AddSourceTerm_CCVar_FullStep()?
If this is indeed a bug, I assume it indicates that you haven't tested the case with MHM on and MHD off. Please test it.
hyschive
left a comment
hyschive
left a comment
There was a problem hiding this comment.
@ChunYen-Chen @technic960183 Comments added. There seem to be a few bugs related to MHM w/o MHD, as well as in some example codes. Please double-check and run additional tests if needed.
| // real div_V[3]; | ||
| // for (int d=0; d<3; d++) | ||
| // { | ||
| // div_V[d] = (real)0.5 * ( g_PriVar_Half[DENS+d][idx_hf + didx_hf[d]] - | ||
| // g_PriVar_Half[DENS+d][idx_hf - didx_hf[d]] ); | ||
| // } // for (int d=0; d<3; d++) |
There was a problem hiding this comment.
Just a question. I don't think that, in general, it will yield identical results as the original CR implementation here, since here it computes div(V) from velocity directly instead of mass flux over density. Do you agree? You may want to add a comment to clarify that.
| // Description : Add source term on the face-centered magnetic field at full-step update | ||
| // | ||
| // Note : 1. Shared by both MHM and MHM_RP | ||
| // 2. Invoked by CPU/CUFLU_FluidSolver_MHM() | ||
| // 3. Must be used after MHD_UpdateMagnetic() |
There was a problem hiding this comment.
Do I understand correctly that one should also update the total energy, which also includes B^2/2, via Hydro_AddSourceTerm_CCVar_FullStep() to treat the B field consistently? If you agree, add a corresponding comment here.
|
|
||
| const real dt_dh2 = (real)0.5 * dt / dh; | ||
| const int N_HF_VAR_P1 = N_HF_VAR + 1; | ||
| const int didx_in[3] = { 1, FLU_NXT, SQR(FLU_NXT) }; |
There was a problem hiding this comment.
How about renaming this variable to better reflect the fact that it's for accessing g_ConVar_In[]?
Co-authored-by: Hsi-Yu Schive <hyschive@gmail.com>
|
@ChunYen-Chen Please let me know when this PR is ready for a final review. Thanks! |
3 participants
This PR contains a simple interface that allows users to add their source term in hydro solver.
Supports
MHMandMHM_RP