[Einstein Toolkit] #625: CapretIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points
#625: CapretIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points --------------------------+------------------------------------------------- Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Keywords: CarpetIOHDF5 | --------------------------+------------------------------------------------- The parameter CarpetIOHDF5::output_symmetry_points = yes does not work when CarpetIOHDF5::output_buffer_points = no (the default). This is probably due to the line
if (not output_buffer_points) { exts &= allactive; }
near line 1071 in OutputSlice.cc. I suspect that allactive does not include the symmetry points. The attached parameter file should be run on one processor. You can then use
h5dump -d '/GRID::x it=0 tl=0 rl=0' output_symmetry/x.x.h5
Near the top you will see
DATASET "/GRID::x it=0 tl=0 rl=0" { DATATYPE H5T_IEEE_F64LE DATASPACE SIMPLE { ( 5 ) / ( 5 ) } DATA { (0): 0, 0.1, 0.2, 0.3, 0.4 }
i.e. the first point output is x = 0. For this parameter file, there should be a symmetry point at x = -0.1. If you modify the parameter file to use CarpetIOHDF5::output_buffer_points = yes, then the symmetry point appears in the output.
The workaround is to always use
CarpetIOHDF5::output_buffer_points = yes
if you want to output symmetry points.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by rhaas):
Are you sure about the default? My copy of CarpetIOHDF5 (ET_2011_10) has a default of "yes": {{{ BOOLEAN output_buffer_points "Output refinement buffer points" STEERABLE = ALWAYS { } "yes" }}} which is the correct thing to have since it is backwards compatible and makes the bug less severe. I agree that only outputting symmetry points when buffer points are also output is a bug. I assume the same issue applies to outer bounday points (which if I remember correctly Carpet does not distinguish from symmetry points).
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by hinder):
I was mistaken about the default - the default for output_buffer_points is yes, as you say. The bug is not triggered in the default case then, which is good. It still remains when output_buffer_points is set to no, however.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by rhaas):
I have some simulations where sliced output without buffers (and maybe some other classes missing as well) produces unreadable files (unreadable to VisIt). Ian, did you ever encounter something similar? I'll have to rewrite the whole thing anyway it seems. On the other hand regrid() now has a variable buffers that might be just what I want. Will likely not happen in time for the next release though.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by hinder):
No, I didn't.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by rhaas):
Tanja had similar issues to mine and after some poking around I think I now understand the reason for Tanja's and mine's problems. It turns out that the fix for that likely also fixes your problem, Ian. Attached please find a preliminary patch for CarpetIOHDF5. If you need these options to work right now, please go ahead and apply the patch otherwise I'll do some cleanup (eventually) and try to test some of the corner cases (and remove support for deprecated parameters from new code).
Buffered and unbuffered output are now handled completely separately (which is ugly), with the old, buffered output removing symmetry and ghost points from the exterior if the are not requested, and the buffered output adding them to the active region if they are requested.
As of late the VisIt reader actually uses the cctk_nghostzones information and passes it on the VisIt, so that we should not longer set the cctk_nghostzones attribute to a non-zero number when we remove the ghost points (and have to make sure that there are ghost points if we claim so).
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: review Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------ Changes (by eschnett):
* status: new => review
Comment:
The patch is okay to apply.
Can you also add test cases? Can we even have test cases that check HDF5 output? If not, that should become a feature request.
The parameters out3D_ghosts and out3D_outer_ghosts are deprecated; if it simplifies the logic, we can remove them.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: reviewed_ok Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------ Changes (by eschnett):
* status: review => reviewed_ok
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: reviewed_ok Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by eschnett):
I assumed that the new code is "better" than the old, even if inelegant. If not, please don't apply it.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: reviewed_ok Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------
Comment (by rhaas):
It produces readable output files in at least one case where the old one did not. I have not tested how many cases where the old code used to work it breaks. The dangerous point seems to me having a cctk_nghostzones attribute with 0 value.
There is no possibility for HDF5 output in test suites as far as I know. A possible solution could use the h5diff utility that comes with HDF5. There is already some discussion about using HDF5 in test suites in #808 (though it is mostly concerned with moving the test suites over to HDF5 to possibly save space).
If out3D_XXX are decprecated then the code should be ok right now, the place where I did not use out3d_ghosts was when deciding what value to write into the ghostzones attributes.
The code unfortunately duplicates code in the buffers and no-buffers branches of an if statement. This is not nice (and the reason for the TODO).
I had to fix a bug in the no-buffers routine and added a test case (though it cannot really be used yet, it passes with 0 of 0 files being identical).
There is likely still room for improvement so I am leaving this ticket open.
Applied as hashes a352f39386df and 1bc87fa7222a of CarpetIOHDF5.
#625: CarpetIOHDF5 sliced output does not output symmetry points unless it also outputs buffer points ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: closed Priority: major | Milestone: Component: Carpet | Version: Resolution: fixed | Keywords: CarpetIOHDF5 ---------------------+------------------------------------------------------ Changes (by rhaas):
* status: reviewed_ok => closed * resolution: => fixed
trac@lists.einsteintoolkit.org
-
Einstein Toolkit