- changed status to open
- removed comment
add "read from file" option ot HydroBase's initial_data options
the attached patch adds a value "read from file" to HydroBase's initial_XXX options. This makes it possible to use IOUtils file reader with hydro data.
This is somewhat similar to IDFileADM's extension of ADMBase's options, only we do not have to set any grid scalars.
Needed to be able to reproduce the MHD paper's collapse test since Whisky_RNSID is not public.
Keyword: HydroBase
Comments (19)
-
reporter -
- removed comment
Shouldn't the thorn providing the capability to read data from a file extend these keywords, instead of HydroBase defining them, but not doing anything with them? Imagine what happens when "read_from_file" is specified, but no thorn feels responsible... Also: when a thorn extends it the keyword should be a little more specific than 'read_from_file'. There could be multiple readers.
-
- changed status to resolved
- removed comment
-
- changed status to open
- removed comment
The thorn reading the data is IOUtil. It should not know anything about HydroBase or ADMBase.
Currently, it is not possible to use IOUtil's file reader with HydroBase, because HydroBase insists on setting up some kind of initial data, always overwriting what one may have read in with the file reader. This patch prevents this overwriting, allowing the file reader to work.
The file reader API may not be ideal, but it is the standard in Cactus. HydroBase should support it.
The proposed patch is equivalent to what ADMBase does.
-
- changed status to open
- removed comment
-
- changed status to open
- removed comment
-
- removed comment
Oh, I didn't think of that. Yes, this is delicate - both thorns don't know of another, but in this case in theory should. Could we add the patch as proposed, but also add a runtime check that looks for an active IOUtils thorn, and checks at least that filereader_ID_vars isn't empty - to avoid both abusing the parameter and accidental usage?
-
- removed comment
Maybe the new keyword could be called IOUtil, so that it's clear where the data is supposed to be coming from.
-
reporter - removed comment
I took a closer look at IDFileADM to see what extra checking it provides.
The text "read from file" is taken from EinsteinInitial/IDFileADM's parameter initial_shift. That one has some longer text explaining the parameter:
Read the initial shift using the file reader. Note that this only allows you to read the shift from a file, it does not actually do it. You still have to programme the file reader accordingly.
To avoid users to run with the wrong data, then I wrote a very simple thorn IDFileHydro that extends the parameters and provides some errror checking in ParamCheck the way IDFileADM does. I put the thorn into incoming: https://svn.einsteintoolkit.org/incoming/IDFileHydro/ .
-
reporter - removed comment
Come to think of it, one could also simply include the ParamCheck routine in https://svn.einsteintoolkit.org/incoming/IDFileHydro/src/IDFileHydro_ParamCheck.c (only piece of code) in IDFileHydro in HydroBase which would mean that the "read from file" option is documented in HydroBase and not some obscure helper thorn that people don't know about.
-
reporter - changed status to resolved
- removed comment
Applied the initial patch as rev 62 and 63 of trunk and ET_2013_05 respectively. Will propose larger patch again after the release.
-
reporter - changed status to open
- removed comment
patch to check consistency of parameters. Also '''adds''' the "read from file" option to initial_hydro where it was missing before.
-
reporter - changed status to open
- removed comment
-
- removed comment
The patch misses the file ParamCheck.c
Also, please include a comment in the parameters that thorn IOUtil is the one a user should look for.
-
reporter - removed comment
updated the patch. added "IOUtil" just before "file reader". Ok to apply?
-
- changed status to open
- removed comment
-
reporter - changed status to resolved
- removed comment
Applied as rev 64 of trunk and rev 65 of ET_2013_05.
-
reporter - removed comment
Fixed a type in the variable name for 'HydroBase::Y_e" in rev 72 of HydroBase.
-
reporter - changed status to closed
- edited description
- Log in to comment