Skip to content

Changed time-averaging algorithm to reduce performance impact. #1548

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
Merged
ChristianBauerEng merged 45 commits into from
Feb 25, 2022
Merged

Changed time-averaging algorithm to reduce performance impact. #1548

ChristianBauerEng merged 45 commits into from
Feb 25, 2022

Conversation

@ChristianBauerEng
Copy link
Contributor

@ChristianBauerEng ChristianBauerEng commented Feb 16, 2022 •
edited
Loading

Proposed Changes

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

  • During time-averaging, the weighted sum over previous timesteps is re-used to quickly update the time-average during inner iterations.
  • Square-windowing now does not save the entire sample history, which should eliminate performance degradation when time-averaging is performed over many timesteps.

Related Work

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

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. (UnitTest)
  • I have updated appropriate documentation (Tutorials, Docs Page, config_template.cpp) , if necessary.

pcarruscag and others added 3 commits October 29, 2021 16:28
@pcarruscag
Merge pull request su2code#1416 from su2code/develop
5655d91 
update master
@pcarruscag
Merge pull request su2code#1521 from su2code/develop
72b2fa9 
Update master
@ChristianBauerEng
- CWindowedAverage now has a constructor that accepts a window type, …
b18f7ea 
…which sets the appropriate private field.

- COutput constructor now pre-initializes the ´windowedTimeAverages´ map with ´CWindowAverage` objects.
- CWindowedAverage.addValue() now caches weighted sums from previous timesteps and does not save samples for rectangular windows.
@pr-triage pr-triage bot added the PR: draft label Feb 16, 2022
@lgtm-com
Copy link

lgtm-com bot commented Feb 17, 2022

This pull request introduces 1 alert when merging 11a4e6b into 5b12ac7 - view on LGTM.com

new alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@lgtm-com
Copy link

lgtm-com bot commented Feb 17, 2022

This pull request introduces 1 alert when merging b4a2d7d into 5b12ac7 - view on LGTM.com

new alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

ChristianBauerEng and others added 9 commits February 17, 2022 18:22
@ChristianBauerEng
- CWindowedAverage now has a constructor that accepts a window type, …
89171d4 
…which sets the appropriate private field.

- COutput constructor now pre-initializes the ´windowedTimeAverages´ map with ´CWindowAverage` objects.
- CWindowedAverage.addValue() now caches weighted sums from previous timesteps and does not save samples for rectangular windows.
@ChristianBauerEng
Fixed syntax error in map initialization
5408323
@ChristianBauerEng
Postprocess_HistoryData no longer uses WindowedUpdate to recompute av…
8957d51 
…erages. They are already up to date.
@ChristianBauerEng
Added default constructor to CWindowAverage
81ec046
@ChristianBauerEng
Fixed return type of updateCachedSum()
e51092e
@ChristianBauerEng
Fixed expression in updateCachedSum()
a5e792a
@ChristianBauerEng
Fixed error in windowWidth calculation and added values initializatio…
249e69e 
…n to constructor
@ChristianBauerEng
Added windowing unit test.
c667353
@ChristianBauerEng
Fixed compiler warnung about type norrowing
f53a40b
@lgtm-com
Copy link

lgtm-com bot commented Feb 17, 2022

This pull request introduces 1 alert when merging f53a40b into 5b12ac7 - view on LGTM.com

new alerts:

  • 1 for Comparison of narrow type with wide type in loop condition

@ChristianBauerEng ChristianBauerEng marked this pull request as ready for review February 17, 2022 18:28
ChristianBauerEng and others added 5 commits February 23, 2022 11:23
@ChristianBauerEng @pcarruscag
Whitespace and comment changes from code review
350c3e4 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@ChristianBauerEng @pcarruscag
Create reference instead of copy of historyOutput_Map iterator
a470bc9 
Suggested from code review

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@ChristianBauerEng @pcarruscag
windowedTimeAverages map should (once again) get properly initialized.
3cb7b3f 
As suggested from code review

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@ChristianBauerEng @pcarruscag
Removed unnecessary if-handling of first timestep
a2f14d3 
From code review

Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@ChristianBauerEng @pcarruscag
Use CHECK and Approx() instead of REQUIRE for unit test
c700751 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@pcarruscag
Copy link
Member

pcarruscag commented Feb 23, 2022

Before I forget, add yourself to the AUTHORS.md file please (alphabetical order), either your name or github handle as you prefer.

pcarruscag
pcarruscag reviewed Feb 24, 2022
View reviewed changes
pcarruscag
pcarruscag reviewed Feb 24, 2022
View reviewed changes
pcarruscag
pcarruscag reviewed Feb 24, 2022
View reviewed changes
pcarruscag
pcarruscag reviewed Feb 24, 2022
View reviewed changes
pcarruscag
pcarruscag reviewed Feb 24, 2022
View reviewed changes
pcarruscag
pcarruscag approved these changes Feb 24, 2022
View reviewed changes
@ChristianBauerEng
Copy link
Contributor Author

ChristianBauerEng commented Feb 24, 2022

I hope my somewhat chaotic commits did not cause more work for you than they actually contributed. This was my first time using github pull requests and code editing without IDE (usually I use Visual Studio). On the upside I now have everything set up to make possible future constributions a bit more efficient :-)
I assume you're merging this PR when everything is ready?

@pcarruscag
Copy link
Member

pcarruscag commented Feb 25, 2022

Not at all this is a nice contribution. It's customary to let the author merge. You can also close issue #1545 after merging.

@ChristianBauerEng ChristianBauerEng merged commit 1be5ed2 into su2code:develop Feb 25, 2022
@ChristianBauerEng ChristianBauerEng mentioned this pull request Feb 25, 2022
Closed
@TobiKattmann TobiKattmann mentioned this pull request Apr 11, 2022
Merged
5 tasks
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.

3 participants

@ChristianBauerEng @TobiKattmann @pcarruscag