Options for appending iteration number to filenames (keep solution snapshots) #1465

bigfooted · 2021-12-06T15:32:01Z

Proposed Changes

Give a brief overview of your contribution here in a few sentences.
WRT_RESTART_OVERWRITE= NO
WRT_VOLUME_OVERWRITE= NO
WRT_SURFACE_OVERWRITE= NO
will create, every WRT_FREQ iterations, output files with the iteration number appended. This is additional to the normal output, so the restart.dat, etc files will always contain the most recent results.

Other features that we can add:
WRT_RESTART_MAX_FILES_TO_KEEP= 5
will keep only the 5 most recently written restart_xxxx.dat files, throwing away older ones.

Related Work

Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.
related to the issue #1463

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 the '-Wall -Wextra -Wno-unused-parameter -Wno-empty-body' compiler flags, or simply --warnlevel=2 when using meson).
My contribution is commented and consistent with SU2 style.
[] 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.

bigfooted · 2021-12-06T15:32:15Z

Currently I have added the keywords WRT_RESTART_OVERWRITE and WRT_VOLUME_OVERWRITE. I think it is best to switch this on/off per type of file because of the large amount of data. There is also inner iterations and outer iterations that we need to take into account when appending the iteration number. Anything else? How to generalize this?

pcarruscag

Like this, this will probably not be compatible with any other feature of SU2, what about:

Never do it for unsteady;
Instead of outputting with the iteration appended, output with the normal name. However, if the file already exists, rename (move) the existing file with the "previous" iteration number, and then continue with the normal output.

This way none of the existing feature should break, and we get the desired feature of having checkpointed files we can go back to if the solver blows up?

bigfooted · 2021-12-07T20:43:28Z

Thanks for the comments!

* Never do it for unsteady;

I saw that too, good idea!

* Instead of outputting with the iteration appended, output with the normal name. However, if the file already exists, rename (move) the existing file with the "previous" iteration number, and then continue with the normal output.

This approach does mean that the file sequence is always behind by OUTPUT_WRT_FREQ. We could also create a symbolic link where restart.dat points to the latest restart_xxx.dat if the existence of restart.dat is necessary for existing workflows.

pcarruscag · 2021-12-07T21:08:33Z

You can store the iteration number for the next renaming so that the sequence is in sync.
I would rather not rely on symbolic links to ensure we don't run into issues with OS or file system quirks. For maximum peace of mind :)

… into feature_file_writing

bigfooted · 2021-12-17T11:11:44Z

OK, I focus on the binary restart for now. What I do now is that with write frequency of 10, at iteration 20 I rename the restart.dat to restart_0010.dat etc. and then at the final iteration 100 I rename the old restart.dat from iteration 90 to restart_0090.dat, I write the new restart.dat. and then I write it a second time, naming it restart_0099.dat.

lgtm-com · 2021-12-18T17:07:22Z

This pull request introduces 1 alert when merging 08d75ff into 4f85841 - view on LGTM.com

new alerts:

1 for Empty branch of conditional

bigfooted · 2021-12-19T07:45:30Z

OK, code is now functional and cleaned up. If the approach for binary restart is fixed, I can will extend it to other outputs. I think the most interesting are the outputs that are used for visualization.

Common/src/CConfig.cpp

SU2_CFD/src/output/COutput.cpp

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

…nded

… into feature_file_writing

bigfooted · 2022-01-05T09:44:13Z

Simplest is to just copy the saved file again, with an iteration number appended. No keeping track of what the iteration number at the previous write was, no copying of the final saved file with manually appending the final iteration number, no lag of WRT_FREQ in the availability of the restart_xxx.dat file.

It does mean that we do a copy instead of a rename. But I think this is the better (because the simplest) solution. I will extend it to the other file options.

pcarruscag · 2022-01-05T12:28:01Z

SU2_CFD/src/output/COutput.cpp

+        if (!config->GetWrt_Restart_Overwrite())
+          (*fileWritingTable) << "SU2 restart with iteration" << filename_iter ;
+
+        (*fileWritingTable) << "SU2 restart" << fileName + CSU2BinaryFileWriter::fileExt;


Before you apply this to the other types, put this block in a private function, to which you pass part of the description ("SU2 restart") and the file extension.

SU2_CFD/src/output/COutput.cpp

pcarruscag · 2022-01-07T22:04:40Z

Common/src/grid_movement/CSurfaceMovement.cpp

                FFDBox[iFFDBox]->SetParaview(geometry, iFFDBox, false);
              }
-            } else if (isTecplot(static_cast<ENUM_OUTPUT>(FileFormat[iFile]))) {
+            } else if (isTecplot(FileFormat[iFile])) {


pcarruscag · 2022-01-07T22:10:09Z

SU2_CFD/src/output/filewriter/CSU2MeshFileWriter.cpp

+  /*--- We append the pre-defined suffix (extension) to the filename (prefix) ---*/
+  val_filename.append(fileExt);


Common/include/option_structure.hpp

…ll testcases

bigfooted · 2022-01-10T07:57:44Z

The testcases contain many comments for OUTPUT_FILES and TABULAR_FORMAT that were outdated descriptions. I have removed all comments for these keywords in the testcases. Actually all testcases need to be stripped from descriptive comments and keywords that are not used or use the default value. This will be a cleanup chore for another PR.

bigfooted · 2022-01-11T20:31:07Z

paraview multiblock is a special case, since it writes a collection of files to a folder. I have now placed the code that was in COutput::WriteToFile into a separate function in CParaviewVTMFileWriter::WriteFolderData. Because the multiblock files need to know abound the mesh and boundary conditions, it needs CConfig and CGeometry as input. But I think all this multiblock code is best placed in this file instead of in COutput.

Common/include/option_structure.hpp

Common/src/CConfig.cpp

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

pcarruscag

LGTM the feature and the other config fixes.
In the future remember to use the su2code repo for your branches, otherwise it's a pain if someone wants to checkout the branch...

pcarruscag · 2022-01-13T09:47:19Z

Common/src/CConfig.cpp

  /* DESCRIPTION: Volume solution files */
  addEnumListOption("OUTPUT_FILES", nVolumeOutputFiles, VolumeOutputFiles, Output_Map);
-
+  


Suggested change

pcarruscag · 2022-01-13T09:47:59Z

Common/src/CConfig.cpp

+    filename = GetUnsteady_FileName(filename, timeIter, ext);
  }
-
+  


Suggested change

pcarruscag · 2022-01-13T09:56:13Z

SU2_CFD/src/output/filewriter/CParallelFileWriter.cpp

+/*--- We append the pre-defined suffix (extension) to the filename (prefix) ---*/
+val_filename.append(fileExt);


Suggested change

/*--- We append the pre-defined suffix (extension) to the filename (prefix) ---*/

val_filename.append(fileExt);

/*--- We append the pre-defined suffix (extension) to the filename (prefix) ---*/

val_filename.append(fileExt);

pcarruscag · 2022-01-13T09:57:41Z

config_template.cfg

-%  SURFACE_PARAVIEW_BINARY, MESH, RESTART_BINARY, RESTART_ASCII, CGNS, SURFACE_CGNS, STL)
+% Possible formats : (TECPLOT_ASCII, TECPLOT, SURFACE_TECPLOT_ASCII,
+%  SURFACE_TECPLOT, CSV, SURFACE_CSV, PARAVIEW_ASCII, PARAVIEW_LEGACY, SURFACE_PARAVIEW_ASCII,
+%  SURFACE_PARAVIEW_LEGACY, PARAVIEW, SURFACE_PARAVIEW, RESTART_ASCII, RESTART, CGNS, SURFACE_CGNS, STL_ASCII, STL)


Suggested change

% SURFACE_PARAVIEW_LEGACY, PARAVIEW, SURFACE_PARAVIEW, RESTART_ASCII, RESTART, CGNS, SURFACE_CGNS, STL_ASCII, STL)

% SURFACE_PARAVIEW_LEGACY, PARAVIEW, SURFACE_PARAVIEW, RESTART_ASCII, RESTART, CGNS, SURFACE_CGNS, STL_ASCII, STL_BINARY)

* remove cout line for paraviewVTMFileWriter * added review suggestions of #1465

initial implementation of OVERWRITE=NO for saved files

074aa6a

pcarruscag reviewed Dec 7, 2021

View reviewed changes

bigfooted and others added 4 commits December 10, 2021 12:56

Merge branch 'develop' into feature_file_writing

1520715

copy restart to restart_xxx

2149d2f

Merge branch 'feature_file_writing' of https://github.com/bigfooted/SU2…

ed3d0c7

… into feature_file_writing

some small cleanup

5513be4

bigfooted added 2 commits December 17, 2021 12:39

remove unused variable

ca5514c

small cleanup - removed cout statements

08d75ff

fix bailout when unsteady

558d542

pcarruscag reviewed Dec 19, 2021

View reviewed changes

bigfooted and others added 5 commits December 20, 2021 23:32

Update Common/src/CConfig.cpp

cc5ee7b

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

Update Common/src/CConfig.cpp

6755afd

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

simplify, just copy the regular saved file with iteration number appe…

a714b0a

…nded

Merge branch 'feature_file_writing' of https://github.com/bigfooted/SU2…

4498eb3

… into feature_file_writing

Merge branch 'develop' into feature_file_writing

6e8d069

pcarruscag reviewed Jan 5, 2022

View reviewed changes

bigfooted added 6 commits January 5, 2022 15:13

use outer iterations for multizone

dc35d9c

modified filewriter class

fd38b44

switched to enum class for enum_output

0e19a3c

consistent naming of string valFileName

9aafca2

consistent naming of string valFileName -> val_filename

6639b2e

consistent naming of string valFileName -> val_filename

8cfa0cc

pcarruscag reviewed Jan 7, 2022

View reviewed changes

bigfooted added 2 commits January 10, 2022 08:49

removed comment lines for option TABULAR_FORMAT and OUTPUT_FILES in a…

119b6aa

…ll testcases

Merge branch 'develop' into feature_file_writing

ecf4b70

bigfooted added 3 commits January 10, 2022 09:44

changed STL to STL_ASCII in testcases

bfd4b3c

always strip file extension when writing file

7a1bcf0

fixing and rewriting of paraview multiblock output

42b90e5

bigfooted changed the title ~~[WIP] append iteration number to filenames~~ append iteration number to filenames Jan 11, 2022

pr-triage bot added the PR: unreviewed label Jan 11, 2022

add options to config_template.cfg

62e6fc8

bigfooted linked an issue Jan 11, 2022 that may be closed by this pull request

Writing solution data in steady state cases without overwriting them #1463

Closed

bigfooted self-assigned this Jan 11, 2022

Merge branch 'develop' into feature_file_writing

b95b5b4

pcarruscag reviewed Jan 12, 2022

View reviewed changes

Common/include/option_structure.hpp Outdated Show resolved Hide resolved

pcarruscag reviewed Jan 12, 2022

View reviewed changes

Common/src/CConfig.cpp Outdated Show resolved Hide resolved

pcarruscag reviewed Jan 12, 2022

View reviewed changes

Common/src/CConfig.cpp Outdated Show resolved Hide resolved

pcarruscag changed the title ~~append iteration number to filenames~~ Options for appending iteration number to filenames (keep solution snapshots) Jan 12, 2022

pcarruscag added the changelog:feature label Jan 12, 2022

bigfooted and others added 4 commits January 12, 2022 22:16

Update Common/include/option_structure.hpp

b522c31

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

Update Common/src/CConfig.cpp

ca06c4e

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

Update Common/src/CConfig.cpp

f9e5387

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>

make filename_iter const

84de9da

pcarruscag approved these changes Jan 13, 2022

View reviewed changes

pr-triage bot added PR: reviewed-approved and removed PR: unreviewed labels Jan 13, 2022

bigfooted merged commit 36be2b8 into su2code:develop Jan 13, 2022

pr-triage bot added PR: merged and removed PR: reviewed-approved labels Jan 13, 2022

bigfooted added a commit that referenced this pull request Jan 13, 2022

remove cout line for paraviewVTMFileWriter (#1491)

7cfe302

* remove cout line for paraviewVTMFileWriter * added review suggestions of #1465

		/--- We append the pre-defined suffix (extension) to the filename (prefix) ---/
		val_filename.append(fileExt);

		/* DESCRIPTION: Volume solution files */
		addEnumListOption("OUTPUT_FILES", nVolumeOutputFiles, VolumeOutputFiles, Output_Map);

	% SURFACE_PARAVIEW_LEGACY, PARAVIEW, SURFACE_PARAVIEW, RESTART_ASCII, RESTART, CGNS, SURFACE_CGNS, STL_ASCII, STL)
	% SURFACE_PARAVIEW_LEGACY, PARAVIEW, SURFACE_PARAVIEW, RESTART_ASCII, RESTART, CGNS, SURFACE_CGNS, STL_ASCII, STL_BINARY)

Options for appending iteration number to filenames (keep solution snapshots) #1465

Options for appending iteration number to filenames (keep solution snapshots) #1465

Uh oh!

Conversation

bigfooted commented Dec 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Related Work

PR Checklist

Uh oh!

bigfooted commented Dec 6, 2021

Uh oh!

pcarruscag left a comment

Choose a reason for hiding this comment

Uh oh!

bigfooted commented Dec 7, 2021

Uh oh!

pcarruscag commented Dec 7, 2021

Uh oh!

bigfooted commented Dec 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgtm-com bot commented Dec 18, 2021

Uh oh!

bigfooted commented Dec 19, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bigfooted commented Jan 5, 2022

Uh oh!

pcarruscag Jan 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pcarruscag Jan 7, 2022

Choose a reason for hiding this comment

Uh oh!

pcarruscag Jan 7, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bigfooted commented Jan 10, 2022

Uh oh!

bigfooted commented Jan 11, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pcarruscag left a comment

Choose a reason for hiding this comment

Uh oh!

pcarruscag Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

pcarruscag Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

pcarruscag Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

pcarruscag Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bigfooted commented Dec 6, 2021 •

edited

Loading

bigfooted commented Dec 17, 2021 •

edited

Loading