- marked as proposal
add ReadInterpolate thorn to Einstein Toolkit
ReadInterpolate is A FileReader like thorn that uses InterolateLocalUniform to interpolate the data read in onto the new grid.
itis available from https://github.com/rhaas80/ReadInterpolate.git and has been used in publications (https://arxiv.org/abs/2003.06043).
Comments (22)
-
reporter -
reporter - edited description
-
reporter @Yosef Zlochower will review this contribution.
-
reporter -
assigned issue to
-
assigned issue to
-
The code seems to require that cctk_lsh[…] = cctk_ash[…] How restrictive is this in practice?
-
reporter Not a whole lot of restriction given that other old thorns do the same. Given that this is a new thorn thorn, it should not do so. I will push a fix.
-
reporter It no longer assumes lsh == ash as of git hash a9e3665 "ReadInterpolate: do not assume lsh == ash" of ReadInterpolate
-
reporter - changed status to open
-
reporter @Yosef Zlochower it would be good to add the thorn to the master thornlist soon so that it can mature a bit and be used / compiled on multiple clusters. This does not signify acceptance yet, just that is is harmless enough to be compiled by default.
In your function as the reviewer, do you judge the code in the thorn to be well written enough to enable it in the master thorn list?
-
reporter @Yosef Zlochower please add any comments about the thorn. Any comments are fine, from places where to code is unclear, to dead code, to missing / incorrect comments, to missing documentation or tests or demos.
-
Yes, please add it the the master thornlist. I think the only outstanding issues is that there needs to be documentation added and a testcase constructed that shows the output values are as expected. For the latter, I was thinking of proposing filling fields with low-order polynomials and then determining if the interpolated values match the polynomials to roundoff precision
-
reporter Tentatively added to master thornlist in git hash 09cc7d4 "einsteintoolkit.th: add ReadInterpolate to master thorn list" of manifest based on positive feedback in https://bitbucket.org/einsteintoolkit/tickets/issues/2416/add-readinterpolate-thorn-to-einstein#comment-58160629
-
reporter I added documentation and a synthetic test case that shows that interpolation is indeed working correctly.
-
Are you using time interpolation in the testsuite?
-
reporter I removed interpolation in time, so the testsuite does not test it.
The original code was (very) wrong (and cannot have ever worked).
I coded up a version that wold stand a chance of working in a branch "rhaas/timerinterp" (just pushed to GitHub):
https://github.com/rhaas80/ReadInterpolate/tree/rhaas/timerinterp
but that one will not work either right now due to the CarpetIOHDF5 checkpoint files (the only one with multiple timelevels which would make interpolation in time possible) lacking information what time the actual time levels actually correspond to. There is not enough information present in the files to deduce what time it is based on the cctk_time attribute that is stored, the delta_time attribute in global attributes and the refinement level information, at least not without making assumptions about time refinement factors. So in the end I
shelved time interpolation. If someone needs it and is willing to spend some time working with me testing it the branch can go in as a feature update sometime after the release once it has been actually used in production.The code can still read in multiple time levels but will only do interpolation in space, requiring that the time step is the same in the source and target simulation (which more or less limits its usefulness to going from a Cartesian to a Llama simulation or so).
-
reporter The synthetic test data tests reading on multiple time levels but does not use time interpolation.
-
reporter @Yosef Zlochower any verdict on the code yet? Once it is reviewed ok, it is put up for a final vote in the next ET call and the formally included in the ET.
-
My only concern was that the synthetic data function is an explicit function of time, but my understanding is that time interpolation doesn’t work.
-
reporter Right, the synthetic data is a function of time so that when i read in three time levels (without interpolation in time) I can actually check that the code did not incorrectly read in data from the dataset for timelevel 0 into all three levels. Reading this now, I of course wonder if the option
ReadInterpolate::read_only_timelevel_0
shouldn’t be removed since (I think) it is already included in the optionsCarpet::init_each_timelevel
(equivalent toread_only_timelevel_0
being false) andCarpet::init_fill_timelevels
(equivalent toread_only_timelevel_0
being true).I will put up ReadInterpolate for vote then. This week with an announcement that there will be a vote next week.
-
reporter Proposed for final review on the mailing list: http://lists.einsteintoolkit.org/pipermail/users/2020-September/007602.html
-
reporter Accepted for inclusion in: http://lists.einsteintoolkit.org/pipermail/users/2020-September/007608.html
Included in einsteininitial in Applied as git hash 56a96c98 "ReadInterpolate: spellcheck and USize spelling in docs" of einsteininitialdata
-
reporter - changed status to resolved
Included. Thank you for rewviewing, @Yosef Zlochower
- Log in to comment