#1478: Prim2ConCells using general Coordinates -------------------------------+-------------------------------------------- Reporter: tl806@… | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Other | Version: development version Keywords: | -------------------------------+-------------------------------------------- Whilst trying to get Llama to work with my simulations in MHD, I had to make the attached changes to Prim2ConM.
I also noticed a possible error with the cells function for the Hydro case, where you use the general coordinates metric, g11 within the function, but not vup.
#1478: Prim2ConCells using general Coordinates --------------------------------+------------------------------------------- Reporter: tl806@… | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Other | Version: development version Resolution: | Keywords: --------------------------------+-------------------------------------------
Comment (by rhaas):
llama.diff is not required since velx, vely and velz are actually preprocessor macros that map to vup so the current code already does (using macros) what llama.diff proposes to do.
I cannot find where llamaM.diff applies. The current trunk version of GRHydro ( rev 586) no longer uses Fortran pointers but instead uses Cray pointers. Unfortunately a grep for {{{ grep -F 'gyy(i,j,k),gyz(i,j,k),gzz(i,j,k)' *.F90 }}} does not find it anywhere (other than the GRHydro_Prim2ConAM.F90) files.
Would you be able to verify that the issue still exists in the current trunk version of GRHydro (rev 586, svn checkout https://svn.einsteintoolkit.org/cactus/EinsteinEvolve/GRHydro/trunk/ GRHydro), please?
Given how much code there is, possibly I am looking at the wrong spot.
#1478: Prim2ConCells using general Coordinates --------------------------------+------------------------------------------- Reporter: tl806@… | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Other | Version: development version Resolution: | Keywords: --------------------------------+-------------------------------------------
Comment (by tl806@…):
Ah, I see my mistake. LlamaM.diff applies to the Gauss release, not the development version.
Though I do believe the same mistake may be in the current trunk version. The problem was in the function "Primitive2ConservativeCellsM" and "Primitive2ConservativePolyCellsM". The subroutine maps the variable vup and Brim, but when it calls the main Prim2Con function, it passes it velx, and Bvecx not vup and Bprim.
Unless this is no longer needed due to the other changes that are included in the trunk version.
#1478: Prim2ConCells using general Coordinates --------------------------------+------------------------------------------- Reporter: tl806@… | Owner: Type: defect | Status: new Priority: minor | Milestone: Component: Other | Version: development version Resolution: | Keywords: --------------------------------+-------------------------------------------
Comment (by rhaas):
Very good. Much relieved to hear that we did not have a bug in the code after all.
If you go to the top of the file (GRHydro_Prim2ConM.F90) then there area set of C preprocessor macros that define velx(i,j,k) as vup(i,j,k,1) etc. If you are familiar with the Whisky code then there is a change in notation in GRHydro since HydroBase (which provides the primitives) does not use velx, vely and velz but rather just vel(i,j,k,D) where D is 1,2,3 for x,y,z. Since some authors like velx better we often have defines for velx at the top of the files and use those instead. Since the define map velx to vup, I believe that indeed the current code is correct (though maybe confusing).
#1478: Prim2ConCells using general Coordinates --------------------------------+------------------------------------------- Reporter: tl806@… | Owner: Type: defect | Status: closed Priority: minor | Milestone: Component: Other | Version: development version Resolution: wontfix | Keywords: --------------------------------+------------------------------------------- Changes (by rhaas):
* status: new => closed * resolution: => wontfix
Comment:
The first issue exists in Gauss and is already fixed in trunk. Given that we are about to release a new version very soon, porting the fix back to Gauss seems to be not needed (if you'd like to do so yourself then you have to do "svn merge https://svn.einsteintoolkit.org/cactus/EinsteinEvolve/GRHydro/trunk -r 585:586", this will create conflicts which you have to fix).
The second reported issue seems to fine as far as I can tell.
trac@lists.einsteintoolkit.org
-
Einstein Toolkit