<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, May 4, 2014 at 4:39 PM, Roland Haas <span dir="ltr"><<a href="mailto:rhaas@tapir.caltech.edu" target="_blank">rhaas@tapir.caltech.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<br>
Hello all,<br>
<br>
due to some issues we have with recovering checkpoints I find myself<br>
reading CarpetIOHDF5's Output.cc and Input.cc files.<br>
<br>
In particular lines 849--860 of Output.cc:<br>
849 ivect const ioffsetdenom = bbox.stride();<br>
850 HDF5_ERROR (attr = H5Acreate (dataset, "ioffsetdenom", \<br>
H5T_NATIVE_INT,<br>
851 dataspace, H5P_DEFAULT));<br>
852 HDF5_ERROR (H5Awrite (attr, H5T_NATIVE_INT, &ioffsetdenom[0]));<br>
853 HDF5_ERROR (H5Aclose (attr));<br>
854 ivect const ioffset =<br>
855 ((bbox.lower() % bbox.stride()) + bbox.stride()) % \<br>
bbox.stride();<br>
856 HDF5_ERROR (attr = H5Acreate (dataset, "ioffset", H5T_NATIVE_INT,<br>
857 dataspace, H5P_DEFAULT));<br>
858 HDF5_ERROR (H5Awrite (attr, H5T_NATIVE_INT, &ioffset[0]));<br>
859 HDF5_ERROR (H5Aclose (attr));<br>
860 ivect const iorigin = (bbox.lower() - ioffset) / bbox.stride();<br>
which as far as I can tell always just sets ioffsetdenom to the stride.<br>
<br>
On the other hand, Input.cc in line 1399-1400:<br>
1399 assert (all (stride % patch->ioffsetdenom == 0));<br>
1400 ivect lower = patch->iorigin * stride + patch->ioffset * stride / \<br>
patch->ioffsetdenom;<br>
only requires stride to be a multiple of ioffsetdenom (rather than<br>
being identical).<br>
<br>
Finally if I read the code correctly then if actually stride !=<br>
ioffsetdenom then the code produces incorrect results since Input.cc<br>
would compute lower as:<br>
<br>
lower = bbox.lower() - ioffset + patch->ioffset * stride / \<br>
patch->ioffsetdenom<br>
<br>
(substituting for patch->iorigin) which only ever evaluates to the<br>
(presumably correct) bbox.lower() if stride == ioffsetdenom.<br>
<br>
Any thoughts?<br></blockquote><div><br></div><div>With vertex centring, the offset denominator is always 1, and this is how CarpetIOHDF5 was originally developed. There may be a bug in handling the denominator, which is only relevant for cell centring. I would also expect that most of the region descriptions calculated here are later ignored, in favour of those calculated by CarpetRegrid2 and CarpetLib. The region descriptions that survive should be the superregions as stored in the grid structure.</div>
<div><br></div><div>Unfortunately, the attributes in CarpetIOHDF5's data format are not well documented. Since these attributes determine whether simulation data can be read back in months or years later, there is a strong pressure to remain backward and bug compatible, and rather to introduce a new attribute than to correct one that is set wrong; presumably, code that reads these attributes (if any) will already deal with these bugs.</div>
<div><br></div><div>In this case, it seems that ioffset is the offset between the actual origin of the grid and the location of the first data point, apparently measured in units of the level's grid spacing. If so, the calculation would be correct: the stride is the distance between two grid points of this level (as measured in CarpetLib's internal integer coordinate system), and ioffset is calculated as the region's lower boundary modulo this stride. (This modulo calculation is slightly complicated by C++'s rules for the module operator for negative numbers.) The expression ioffset / ioffsetdenom, interpreted as real number, is thus non-negative, strictly less than one, and the particular grid spacing used by CarpetLib's internal integer coordinate system is irrelevant since it cancels.</div>
<div><br></div><div>The expression ioffset / ioffsetdenom should be interpreted as real number, i.e. this fraction could be arbitrarily reduced, although CarpetIOHDF5 doesn't do that. When reading, the actual value of ioffsetdenom does not matter, since only the fraction ioffset / ioffsetdenom should be evaluated; however, since integer arithmetic is used, the newly used denominator needs to be sufficiently large to hold the (reduced) fraction.</div>
<div><br></div><div>It seems that the assert in line 1399 is too strict if the fraction ioffset / iofsetdenom can reduced.</div><div><br></div><div>I do not understand what you want to say in the paragraph starting with "Finally". bbox.lower is the location of the first grid point of a region; the actual domain covered by the region would be 1/2 grid spacing larger if cell centring is used. However, I don't know whether this is actually the calculation performed here.</div>
<div><br></div><div>-erik</div><div><br></div></div>-- <br>Erik Schnetter <<a href="mailto:schnetter@cct.lsu.edu" target="_blank">schnetter@cct.lsu.edu</a>><br><a href="http://www.perimeterinstitute.ca/personal/eschnetter/" target="_blank">http://www.perimeterinstitute.ca/personal/eschnetter/</a>
</div></div>