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 output names of unsteady simulation #2418

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

rois1995
Copy link
Contributor

Proposed Changes

It solves two main issues in unsteady simulations:
1- if the history file is not updated during the simulation (ex. setting inner_iter = 0) the time iteration is not updated in the file output names.
2- mesh file output does not have the time iteration at the end.

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 used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • 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.

@rois1995
Copy link
Contributor Author

I would also try to add the writing of the mesh in the CGNS format, although this can be done in another PR.

@bigfooted
Copy link
Contributor

LGTM

Comment on lines 481 to +482
if (fileName.empty())
fileName = volumeFilename;
fileName = config->GetFilename(volumeFilename, "", curTimeIter);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if the filename is empty, the time iteration is added, but if the filename is not empty, the iterations were always already added before to fileName, is that it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially I have followed all the other cases. Without appending the time iter at the end of the file name it just overwrites the same file at each unsteady iteration.

Comment on lines 392 to 393
/*--- Set current time iter even if history file is not written ---*/
curTimeIter = config->GetTimeIter();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If curTimeIter is a member variable, why are you having to set its value here?
It was already used in line 418/421.
So do we have the same problem with innerIter and outerIter? Can you fix this closer to the current place where these iteration counters are set? (history file judging from the comment)

Copy link
Contributor Author

@rois1995 rois1995 Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem only arises when I set the inner iterations to be 0 in an unsteady dual time-stepping simulation. I guess that, since in this case the history file is not updated, the curTimeIter is not updated and remains 0 when writing the output files. In all other cases where the history file is written it is not necessary. I guess I could use an if-statement for that but I don't think that it is necessary. I don't know if the problem persist with the outer iter, I will check (I guess it only matters during a steady simulation when (WRT_*_OVERWRITE is set to YES in the config file). However, in that case if the iterations are set to 0 then the simulation terminates right away, thus there should be no problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but please look for a single place where to put setting these variables, potentially moving where they are set at the moment.

Copy link
Contributor Author

@rois1995 rois1995 Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I missed that request before. I have created a setCurTimeIter function in the COutput class and I call it when the Solve function is called. I have checked also for multi-zone problems and it works fine. I can remove the TimeIter variable from all the other functions of the COutput class where the curTimeIter was set equal to it to clean up the code if needed.

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.

3 participants