GRHydro updates

Issue #1415 closed
Roland Haas created an issue

accumulated GRHydro changes since before the CGWAS school (July 22nd).

Includes the PPM changes from the workshop. Does not yet include the staggered vector potential work since this seems to be still in early stages.

Will commit after Thursday unless objections are raised.

Keyword: GRHydro

Comments (22)

  1. Frank Löffler
    • removed comment

    Partial review: 0001 looks good, although intendation looks a bit odd in places (e.g. see calculation of beta1).

  2. Frank Löffler
    • removed comment

    Why is 0002 necessary? I don't like parameters with a name *_hack, for obvious reasons.

  3. Roland Haas reporter
    • removed comment

    Replying to [comment:3 knarf]:

    Why is 0002 necessary? I don't like parameters with a name *_hack, for obvious reasons. There is already an octant_hack :-) . The option prevents some aborts in prim2con which aborts for hot eos if some EOS calls failed. Essentially it affects points in the symmetry region that will be filled in by the symmetry operation later on. Affected points could be eg on the coarse grid and only be filled in by the symmetry call after restriction.

    The "correct" fix to this seems to me to have a prim2confailed mask similar to con2primfailed (or re-used that mask) and only abort if the mask is set after restriction.

    Until then the _hack parameters provide a workaround that lets on run some runs that otherwise fail.

    I am not wedded to this parameter though (mostly I just like git and svn to be in sync as much as possible).

  4. Roland Haas reporter
    • removed comment

    Replying to [comment:2 knarf]:

    Partial review: 0001 looks good, although intendation looks a bit odd in places (e.g. see calculation of beta1). Thank you for the review. Indentation in this file seems to vary between 1 space and 3 spaces. Within each block it is consistent though (though the text in the comments does not align with the code admittedly). I'll commit as is with the other once they are reviewed (one way or the other).

  5. Frank Löffler
    • removed comment

    Replying to [comment:4 rhaas]:

    There is already an octant_hack :-) . The option prevents some aborts in prim2con which aborts for hot eos if some EOS calls failed. Essentially it affects points in the symmetry region that will be filled in by the symmetry operation later on.

    Why don't we not first fill in the points with the correct primitives and do prim2con then? If it still fails then, it should not only fail in the symmetry zones, but also outside, as the symmetry zones should be exact copies of the values outside, and prim2con is a local operation.

  6. Frank Löffler
    • removed comment

    0007 should call GRHydro_RefinementLevel within the scheduler instead. No need to repeat this expression.

  7. Frank Löffler
    • removed comment

    0008: Any reason why the keyerr couldn't be allocated by Cactus instead? I would prefer that. Also, if you really must allocate it there, use cctk_ash[]. Also: please remove the comment that talks about that malloc would be better when you now use malloc.

  8. Frank Löffler
    • removed comment

    Why does 0011 save time?

    Only cosmetics: why two nested loops for evolve_mhd and transport_constraints?

  9. Frank Löffler
    • removed comment

    0012: If this disables a mistake, why isn't the mistake then not taken out of the code entirely, instead of only protecting it using the cpp?

  10. Frank Löffler
    • removed comment

    0018: It would be nice to decide when this code should be completely removed. Just commenting it messes up the source and shouldn't be done if the plan is to dump this entirely in the future.

  11. Roland Haas reporter
    • changed status to resolved
    • removed comment

    Committed 0001, 0004, 0005, 0006, 0007 (renamed GRHydro_reflvel to reflevel, I will not schedule the scheduled routine since I do not know from where con2prim will be called and would have to make very sure that the extra GRHydro_RefinementLevel runs just before, for just a diagnostic output this seems not worthwhole. Much more sensible in that case to simple call Carpet's GetRefinementLevel alias routine), applied 0008 using ash instead of lsh (making this a grid function is awkward since it is only used for one combination of evolution_method, evolve_mhd, evolve_temperature and recon_vars some of which are tested in Fortran code and not in schedule.ccl, 0009, 0010, 0011 removed the nested ifs, re-worded commit message, 0012 removing lines of code, 0013, 0014 to 0017. These are revs 563 - 579 of GRHydro.

  12. Roland Haas reporter
    • removed comment

    Applied 0018 as is. This code is under heavy development. I do not want to create huge conflicts with the development code by taking out pieces of code at random.

  13. Log in to comment