- removed comment
CarpetIOASCII modification causes tests to fail
Issue #690
closed
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?
Keyword: testsuites
Keyword: CarpetIOASCII
Comments (13)
-
-
reporter
- removed comment
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.
-
- removed comment
I corrected the code that emitted the erroneous newline.
-
reporter
- removed comment
The attached patch ignores just comments rather than both comments and blank lines. OK to apply?
-
reporter
- removed comment
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!).
-
- removed comment
Please apply.
-
- removed comment
Yes, the space->tab change was intentional.
-
- removed comment
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?
-
reporter
- removed comment
Committed in r4773 of the flesh.
-
- removed comment
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.
-
reporter
- removed comment
Yes - tests all pass.
-
- changed status to resolved
- removed comment
-
- edited description
- changed status to closed
- Log in to comment
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.