#1796: Include IllinoisGRMHD into the Toolkit, as an Arrangement ------------------------------------+--------------------------------------- Reporter: zachetie@… | Owner: Zachariah Etienne Type: enhancement | Status: new Priority: major | Milestone: ET_2015_11 Component: EinsteinToolkit thorn | Version: development version Resolution: | Keywords: GRMHD IllinoisGRMHD ------------------------------------+---------------------------------------
Comment (by knarf):
Some thoughts, in the order I found them (not severity):
convert_to_HydroBase: * Most thorns usually have the convention of starting with an upper case letter. Not a must, but these stand out a little because of that. Not a must change of course. * It would be nice to provide a README file. It doesn't need to be long, just look at some examples from other thorns. Not a must, but a would be nice. * Why do you explicitly enable storage for ADMBase and HydroBase variables? Did you find that these thorns do this insufficiently? * You might want to schedule "in INITIAL after HydroBase_Initial", instead of in POSTINITIAL after SetTmunu. SetTmunu is, as far as I can see, also scheduled in Initial, and not in PostInitial, so this 'after' statement is always fulfilled already. * the make.code.defn file contains "Main make.code.defn file for thorn smallbPoyn" * Why do you have it depend on CarpetRegrid2? ID_converter_ILGRMHD: * There is a "shares: driver" statement without actually anything sharing. * Same question about explicit storage of other thorn's variables * You schedule convertion IN HydroBase_Initial, instead of after it, explicitly depending on AFTER statements of a few known ID thorns. This is likely to fail for other ID thorns. Would it be possible to schedule AFTER HydroBase_Initial? * make.code.defn file for thorn IllinoisGRMHD_InitialData? * The random perturbation code would be better in a separate thorn. Wouldn't this be also useful for other purposes than just when using the conversion? both: * There are no testsuites * There is no (at least short) documentation. It doesn't need to be much, given these thorns.