Change default compression_level of vtu output
#14983
Conversation
|
/rebuild |
|
Some tests, in which the output files are compared to reference files from the repo are failing now. This is clear since the compression_level has changed.
What way would be the preferred one? |
|
I would simply adjust the compression level of the tests. |
|
My personal preference would be (i) to adjust the tests, and (ii) clone one of the tests that continues to check the "default" case. In practice, you want to do step (ii) before step (i) because you want to use how the existing tests use the default branch. |
|
Shall I run the full regression tester on this one? |
|
Tests still need to be adjusted and I am not too worried, because Jenkins should cover all of the tests (minus ones that require uncommon dependencies). |
from best_compression to best_speed, as discussed in pull request dealii#14958. The compression flags of the tests is set to best_compression to conserve the current output behavior of the tests.
c-p-schmidt
force-pushed
the
change-default-compression-level-of-vtu
branch
from
bfb0ce5
to
87c98d2
Compare
There was a problem hiding this comment.
I now changed the compression_level of the failing tests to best_compression.
@bangerth: I did not copy a test to test it with best_speed and best_compression, since I realized that there is already the test data_out/data_out_base_compression_levels.cc that is outputting data with all available compression_levels.
| DataOutBase::write_vtu(patches, data_names, vector_data_ranges, flags, out); | ||
| vector_data_ranges; | ||
| DataOutBase::write_vtu( | ||
| patches, data_names, vector_data_ranges, vtu_flags, out); |
There was a problem hiding this comment.
While working on adapting the compression_level for the failing tests, I guess I found a bug in the code. Despite being able to set a compression_level using the GridOutFlags::Vtu, those were ignored here and instead Vtkflags with default settings have been passed to the write_vtu method
There was a problem hiding this comment.
This is too funny. Wait, what day is today?😃
There was a problem hiding this comment.
Ha, that's wild. I guess this comes from a time when we did not actually have separate VtuFlags.
I'll accept this patch as is, but would you mind adding an entry to doc/news/changes/minor that describes this particular problem and fix?
…ompression-level-vtu Document minor change that has been introduced in pull request #14983.
As discussed in #14958 (see e.g. here for on overview of output performance based on the compression_level or here for the suggestion that probably best_speed should also be set as default for the vtu output) I'm proposing to change the default compression_level for the vtu output from
best_compressiontobest_speed.Tagging @tjhei and @drwells as you might be interested.