Place Postprocessor Files Within Respective "postprocessor_output" Subdirectories

[danieldouglas92]

This is something that I don't feel is absolutely necessary, but it is something that I've thought would make sense. I often found that for models that I ran which output topography I would personally make a topography directory and just move the files there, so I figured this change might also be appreciated by others. If this does seem worth while, there are several other postprocessors which could benefit from a similar change.

[bangerth]

This is ok with me, but let's say what others have to say. You'll have to adjust tests as well, and you should check that the test harness doesn't just pick up stored output file in the top level directory of every test, but recurses into sub-directories as well.

[gassmoeller]

Yes, I like it! There are several postprocessors (e.g. also heat flux map) that just flood the output directory with files, and I am much in favor of separating them into different directories. As Wolfgang said you will have to update test results.

[danieldouglas92]

@bangerth @gassmoeller thanks for the quick feedback! I'll go through updating the tests and getting the code for this specific postprocessor in good shape, and then I'll go through and apply the same methodology to the other postprocessors which would benefit from this (heat flux, sealevel, dynamic topography etc.)

[danieldouglas92]

@gassmoeller @bangerth the testers have all passed and I've addressed Rene's comments. I'll be honest I'm not entirely sure how to check if the tester is recursing through all sub-directories, but the tester does do something similar with the particles (particle_output_vtu for example) and solution (hk04_olivine_composite_hydration_prefactor for example) subdirectories within existing tests.

Between the testers not failing and evidence of recursively checking other directories, is that enough to be convinced that the topography directory is being checked? Or is there a better way to confirm that this is happening?

[gassmoeller]

Or is there a better way to confirm that this is happening?

What I like to do is to break the test output on purpose (modify one of the topo files by hand), and check that the test actually fails. Maybe you could do that? If it fails, just undo your commit and let us know.

[gassmoeller]

Looks good to me, if the test I suggested above works (=fails) I think this is ready to merge.

[danieldouglas92]

@gassmoeller @bangerth Only one tester failed (dealii-9.6-v1), the other two testers passed, so seemingly these two testers are not recursively checking all sub-directories.

[gassmoeller]

@danieldouglas92 no that is expected. Only dealii-9.6-v1 actually checks the results of the tests, all the other testers only make sure that the tests are running without crash. That is for the same reason why we need a reference docker container as tester: every deal.II version (and ubuntu version and trilinos version etc.) may produce marginally different results, which would then appear as failing test in our continuous integration pipeline. Therefore, only one of the testers actually compares the test results.

So all is well, remove the last commit and this PR is ready to merge.

[danieldouglas92]

@gassmoeller I rebased and fixed the test output. If it makes sense I can go through and modify the rest of the Postprocessors in this PR? Otherwise I'll open a follow-up PR once this one is merged that does this for the other Postprocessors.

[gassmoeller]

Lets do the other postprocessors in a follow up PR. Keeping PRs small and separate makes it easier to review and figure out changes in the future.

[bangerth]

I'm sorry to take a while to get to this. The check for which files to compare indeed recurses into subdirectories, as I think you've found out above:
https://github.com/geodynamics/aspect/blob/main/tests/CMakeLists.txt#L238-L239
So I think it's all good!