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.

Sign up for GitHub

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

Jump to bottom

Allow different OUTPUT_WRT_FREQ for each output file #1552

Merged
merged 52 commits into from
Mar 29, 2022
Merged

Allow different OUTPUT_WRT_FREQ for each output file #1552

merged 52 commits into from
Mar 29, 2022

Conversation

bigfooted
Copy link
Contributor

@bigfooted bigfooted commented Mar 3, 2022 •
edited
Loading

Proposed Changes

Give a brief overview of your contribution here in a few sentences.

  • add frequency for each filename in OUTPUT_FILES:
    OUTPUT_WRT_FREQ= 10, 100, 50

  • for dual timestepping, write only the restart files twice (for timestep n and n-1) and not the visualisation files.

  • history output accepts fields

  • marker_inlet does not need normalized vectors (removed the error message)

- DONE
The suggestion was to just extend the OUTPUT_WRT_FREQ instead of introducing a new keyword VOLUME_OUTPUT_FREQUENCIES.

Related Work

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

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.

@lgtm-com
Copy link

lgtm-com bot commented Mar 3, 2022

This pull request introduces 25 alerts when merging 6ad89f7 into 3832a8f - view on LGTM.com

new alerts:

  • 23 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 3, 2022

This pull request introduces 25 alerts when merging 63bf74c into 3832a8f - view on LGTM.com

new alerts:

  • 23 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 3, 2022

This pull request introduces 25 alerts when merging 527357d into 2786e53 - view on LGTM.com

new alerts:

  • 23 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2022

This pull request introduces 22 alerts when merging 5a86fb1 into 2786e53 - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@bigfooted
Copy link
Contributor Author

I don't know why there are suddenly all these additional lgtm alerts. Most of them have nothing to do with what I did. I fixed 3 of them.

@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2022

This pull request introduces 22 alerts when merging e8fcf48 into 2786e53 - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2022

This pull request introduces 22 alerts when merging ca466b1 into 2786e53 - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 16, 2022

This pull request introduces 22 alerts when merging 97a1b1d into a288828 - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 16, 2022

This pull request introduces 22 alerts when merging 6ff6acd into c8ccde5 - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 22, 2022

This pull request introduces 22 alerts when merging 5a80ab7 into 4c3311f - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 23, 2022

This pull request introduces 22 alerts when merging 5f0af78 into 4c3311f - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 23, 2022

This pull request introduces 22 alerts when merging fac06fa into 3a5b668 - view on LGTM.com

new alerts:

  • 20 for Resource not released in destructor
  • 2 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Mar 28, 2022

This pull request introduces 2 alerts when merging 3702598 into 3f3cb55 - view on LGTM.com

new alerts:

  • 2 for Comparison result is always the same

bigfooted and others added 8 commits March 28, 2022 16:01
@bigfooted @pcarruscag
Update Common/src/CConfig.cpp
bbd289d 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update Common/src/CConfig.cpp
b0c6328 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update Common/src/CConfig.cpp
3919213 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update Common/src/CConfig.cpp
1496acd 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update Common/src/CConfig.cpp
3a19b74 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update Common/src/CConfig.cpp
78abd91 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update SU2_CFD/src/output/COutput.cpp
9fc419d 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update config_template.cfg
d5bd78b 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
pcarruscag
pcarruscag reviewed Mar 28, 2022
View reviewed changes
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

LGTM fixes below

SU2_CFD/src/output/COutput.cpp Outdated Show resolved Hide resolved
SU2_CFD/src/output/COutput.cpp Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Mar 28, 2022

This pull request introduces 2 alerts when merging d5bd78b into 3f3cb55 - view on LGTM.com

new alerts:

  • 2 for Comparison result is always the same

bigfooted and others added 3 commits March 28, 2022 18:11
@bigfooted @pcarruscag
Update SU2_CFD/src/output/COutput.cpp
531a98c 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted @pcarruscag
Update SU2_CFD/src/output/COutput.cpp
f8f8926 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@bigfooted
modified 3 testcases
ea8e6c5
@bigfooted
Copy link
Contributor Author

Modify one of the testcases to exercise this please (even if we don't have a way to check it is doing the right thing).

I'll update the documentation as well

bigfooted
bigfooted commented Mar 29, 2022
View reviewed changes
Common/src/CConfig.cpp Outdated Show resolved Hide resolved
pcarruscag
pcarruscag reviewed Mar 29, 2022
View reviewed changes
Common/src/CConfig.cpp Outdated Show resolved Hide resolved
Common/src/CConfig.cpp Outdated Show resolved Hide resolved
pcarruscag
pcarruscag approved these changes Mar 29, 2022
View reviewed changes
Copy link
Member

@pcarruscag pcarruscag left a comment

Choose a reason for hiding this comment

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

LGTM

@bigfooted
Copy link
Contributor Author

discadj_incomp_turb_NACA0012_sa now fails with a small difference in residuals. But this case passed the regression test on my machine. I don't see what could have triggered this change.

@pcarruscag
Copy link
Member

#1384

@bigfooted
Copy link
Contributor Author

Thanks, just rerunning the regression seemed to fix the discrepancy. OK, I'll merge, and I'll add another beer to the IOU list!

@bigfooted bigfooted merged commit c100612 into develop Mar 29, 2022
@bigfooted bigfooted deleted the feature_filewriting branch March 29, 2022 11:55
@pcarruscag pcarruscag mentioned this pull request Mar 31, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pcarruscag pcarruscag pcarruscag approved these changes

Assignees
No one assigned
Labels
changelog:feature PR: merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bigfooted @pcarruscag