Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix functionality of 'WRT_*_OVERWRITE' flags. #2036

Merged
merged 4 commits into from
May 21, 2023

Conversation

danielmayer
Copy link
Contributor

@danielmayer danielmayer commented May 17, 2023

Proposed Changes

The flags 'WRT_VOLUME_OVERWRITE', 'WRT_SURFACE_OVERWRITE', and 'WRT_RESTART_OVERWRITE' are not working correctly. This commit fixes this.

The expected behavior is this:
The user specifies what kind of output files should be written using the OUTPUT_FILES flag. These output files can be volume or surface files for visualization like Tecplot or Paraview files, volumetric restart files to be used as initial conditions for another SU2 run, or mesh format files like CGNS or STL files.
Furthermore, the user can choose to not overwrite these files, but rather add the current iteration number to the file name and keep dumping such files during the calculation. This is done using the three WRT_*_OVERWRITE flags mentioned above.

However, the current implementation in develop only works correctly for PARAVIEW_MULTIBLOCK and for RESTART type output files. This is because in the case of PARAVIEW_MULTIBLOCK, the additional file output using the iteration number in the filename, is performed in the switch case block directly.

For all other output files, the switch case block only generates the correct filename with the iteration number appended. After the switch block, it is (wrongly) checked if (and only if) the WRT_RESTART_OVERWRITE flag is set to NO. If so, the additional file is written. Therefore, this logic only works correctly for RESTART files (and PARAVIEW_MULTIBLOCK files, because for that format, the file dump is independent of the file dump after the switch block.

The proposed fix here introduces a local boolean (doWriteIterFile) that acts as a switch for the additional file dump. It is set in the switch case block and afterwards it is checked if this variable is true or false. Based on this value, the file dump is executed or not. Furthermore, the fix introduces and corrects some comments to make the function easier to understand.

Related Work

The flags are already available in the develop branch as config options but they don't work as expected.

PR Checklist

Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.

  • I am submitting my contribution to the develop branch.
  • My contribution generates no new compiler warnings (try with --warnlevel=3 when using meson).
  • My contribution is commented and consistent with SU2 style (https://su2code.github.io/docs_v7/Style-Guide/).
  • I have added a test case that demonstrates my contribution, if necessary.
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp), if necessary.

@pcarruscag
Copy link
Member

Can you describe the bug this is fixing a bit more

@danielmayer
Copy link
Contributor Author

Can you describe the bug this is fixing a bit more

I edited the PR description. Hope it's clear now. Please let know if you need more info.

@pcarruscag
Copy link
Member

I see, I get it now thanks (it sounded like everything was not working :) )

@pcarruscag pcarruscag merged commit 0e12033 into develop May 21, 2023
@pcarruscag pcarruscag deleted the feature_outputfiles_no_overwrite branch May 21, 2023 03:54
@danielmayer
Copy link
Contributor Author

Sweet! Thank you Pedro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants