#690: CarpetIOASCII modification causes tests to fail --------------------------------------+------------------------------------- Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Keywords: testsuites CarpetIOASCII | --------------------------------------+------------------------------------- The following changeset,
changeset: 3377:2f2e12bdf05b tag: tip user: Erik Schnetter schnetter@cct.lsu.edu date: Tue Nov 15 11:59:53 2011 -0500 summary: CarpetIOASCII: Add new "compact" output format
causes the default CarpetIOASCII output format to change. Specifically,
{{{ # iteration 0 # refinement level 0 multigrid level 0 map 0 component 0 time level 0 # column format: 1:it 2:tl 3:rl 4:c 5:ml 6:ix 7:iy 8:iz 9:time 10:x 11:y 12:z 13:data 0 0 0 0 0 0 18 3 0 -0.473684210526316 0.0967741935483871 0 0 }}}
becomes
{{{ # iteration 0 time 0 # time level 0 # refinement level 0 multigrid level 0 map 0 component 0 # column format: 1:it 2:tl 3:rl 4:c 5:ml 6:ix 7:iy 8:iz 9:time 10:x 11:y 12:z 13:data
0 0 0 0 0 0 18 3 0 -0.473684210526316 0.0967741935483871 0 0 }}}
This causes certain test cases (I think those which do not use IO::out_fileinfo = "none") to fail. The problem is not in the modification of the comment lines, whose content is ignored by the test mechanism, but in the addition of a blank line or a comment line (the "time level 0" in the above example). The test mechanism compares files line-by-line, and so becomes out of step if additional comment or blank lines are introduced.
The attached patch modifies the test mechanism to completely ignore the presence of any blank or comment lines. With this patch, the tests bhns_eval, bhns_interp, checkpointML and recoverML now pass again.
Is this solution acceptable? Should the test mechanism be modified in a different way? Or should CarpetIOASCII be modified to produce the old format?
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by eschnett):
I was not aware that my changes modified the number of empty lines in the default output format; this should not be the case. Comment lines should be ignored when comparing output, but empty lines have meaning to gnuplot, and maybe to other tools as well.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by hinder):
OK, so the best thing would be to modify the patch to skip all comment lines completely during the tests, and to modify CarpetIOASCII to omit the extra blank line.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by eschnett):
I corrected the code that emitted the erroneous newline.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by hinder):
The attached patch ignores just comments rather than both comments and blank lines. OK to apply?
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by hinder):
For the record, the original carpetioascii changeset (2f2e12bdf05b) also modified one of the field separators from space to tab. I don't know if this was intentional, but the test mechanism handles this automatically (via perl's "magic" interpretation of the space character as "any whitespace" in the split function!).
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by eschnett):
Please apply.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by eschnett):
Yes, the space->tab change was intentional.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by rhaas):
this might break some tools that actually distinguish between space and tab (cut(1) comes to mind and of course matlab's DLMREAD but that one chokes on the '#' comment anyway). Is the reason for the space->tab change that tabs should separate groups of data elements and space members of the groups?
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by hinder):
Committed in r4773 of the flesh.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by eschnett):
Yes, this is the reason. After this change, the standard and the new compact format are more uniform.
Is this defect now corrected? The test cases pass on my system.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: new Priority: major | Milestone: Component: Carpet | Version: Resolution: | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------
Comment (by hinder):
Yes - tests all pass.
#690: CarpetIOASCII modification causes tests to fail ---------------------+------------------------------------------------------ Reporter: hinder | Owner: eschnett Type: defect | Status: closed Priority: major | Milestone: Component: Carpet | Version: Resolution: fixed | Keywords: testsuites CarpetIOASCII ---------------------+------------------------------------------------------ Changes (by eschnett):
* status: new => closed * resolution: => fixed
trac@lists.einsteintoolkit.org
-
Einstein Toolkit