Skip to content

Hybrid Parallel (AD): Fix Race Conditions #1679

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
jblueh merged 51 commits into from
Jul 9, 2022
Merged

Hybrid Parallel (AD): Fix Race Conditions #1679

jblueh merged 51 commits into from
Jul 9, 2022

Conversation

@jblueh
Copy link
Contributor

@jblueh jblueh commented Jun 22, 2022 •
edited by pcarruscag
Loading

Proposed Changes

@maxaehle and I analyzed SU2 with gcc with the thread sanitizer enabled and fixed various race conditions both in hybrid parallel and hybrid parallel AD code.

The thread sanitizer records of the hybrid regression tests and hybrid regression AD tests are clean now.

Related Work

#1384, this PR works towards fixing it

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.

maxaehle and others added 30 commits May 5, 2022 08:06
@maxaehle
Define SU2_OMP_SINGLE
153b6ef
@maxaehle
Fix concurrent zero-initialization of SetBeta_Parameter::MaxVel2
cf77f4e
@maxaehle
Fix concurrent calls to CConfig::SetGlobalParam in Preprocessing
436f8cc
@maxaehle
Add omp atomic/critical pragmas in CNSSolver::SetTau_Wall_WF
f3959c6
@maxaehle
omp single around SetGlobalParam for some other Preprocessing's
6a0fb5c
@jblueh
Missing end macro.
4934504
@jblueh
Fix race on residual in Solve_b.
a67538e
@jblueh
Explicit resizing of adjoint vector.
afd748f
@maxaehle
ResizeAdjoints() for non-AD builds.
cb9234e
@maxaehle
Fix race condition in CMultiGridIntegration::MultiGrid_Cycle
78ffdc0
@maxaehle
Add barrier before the assignment in ompMasterAssignBarrier.
3e1fa38
@maxaehle
SU2_OMP_CRITICAL access of CVariable::Non_Physical.
f879d56
@maxaehle
SU2_OMP_CRITICAL access of Non_Physical for incompressible Euler
526e455
@maxaehle
OpenMP worksharing in PreprocessBC_Giles
ac8ea30
@maxaehle
Remove nowait clause.
47dae26
@maxaehle
omp single in ComputeSurfaceAreaCfgFile
7d3bf0f
@maxaehle
omp single -> omp master
1859c19
@maxaehle
fix
3723ede
@jblueh
Undo incorrect loop splitting.
4577b30
@jblueh
Safeguard Kind_TimeIntScheme.
54cd32e
@jblueh
Add an option for disabling preacc to tests.
dfe691b
@jblueh
Consistent comment formatting.
8d79a2b
@jblueh
CoDiPack update.
d2c65f9
@jblueh
OpDiLib update.
4da85aa
@jblueh
Update commit hashes.
cdc87cf
@jblueh
Fix hardcoded AD types.
6fab68a
@jblueh
OpDiLib update.
f5f491d
@jblueh
CoDiPack update.
75aaa51
@jblueh
Update test values.
d0b3138
@jblueh
Explicit resizing no longer needed.
dac909a
pcarruscag
pcarruscag reviewed Jun 22, 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.

Awesome, thank you for taking a deep look into these issues 👍

jblueh and others added 13 commits June 28, 2022 10:09
@jblueh @pcarruscag
Update Common/src/linear_algebra/CSysSolve.cpp
a236b5a 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@jblueh
Fix leftover tsan findings in non-AD code.
7cffa8e
@jblueh
Add SU2_OMP_SAFE_GLOBAL_ACCESS variants.
8ed32fc 
Add it to the syntax checker and apply it throughout the code.
@jblueh
Revert "Add an option for disabling preacc to tests."
0b021a7 
This reverts commit dfe691b.
@jblueh @pcarruscag
Update Common/src/linear_algebra/CSysMatrix.cpp
7bd3512 
Co-authored-by: Pedro Gomes <38071223+pcarruscag@users.noreply.github.com>
@jblueh
Further applications of SU2_OMP_SAFE_GLOBAL_ACCESS.
7a2eb43
@jblueh
Merge branch 'fix_race_conditions' of github:su2code/SU2 into fix_rac…
85ebbe0 
…e_conditions
@pcarruscag
Merge branch 'develop' into fix_race_conditions
2378913
@pcarruscag
edge-based strategy for reconstructions
c523946
@jblueh
GetNode for non-contiguous edges.
deda9e3
@jblueh
Use hybrid parallel AD test values from CI pipeline.
a419f8d
@pcarruscag
use the gather ctor of simd array
a501b19
@pcarruscag
pad nodes consistently
cf1a98d
pcarruscag
pcarruscag reviewed Jul 7, 2022
View reviewed changes
@TobiKattmann
Copy link
Contributor

TobiKattmann commented Jul 7, 2022

update testcases, streamwise pin converges well Thanks for checking @pcarruscag :)

@jblueh jblueh linked an issue Jul 9, 2022 that may be closed by this pull request
Hybrid parallel AD regression tests fail sometimes #1384
Closed
@pcarruscag pcarruscag changed the title [WIP] Hybrid Parallel (AD): Fix Race Conditions Hybrid Parallel (AD): Fix Race Conditions Jul 9, 2022
@jblueh jblueh merged commit 3a9f8d6 into develop Jul 9, 2022
@jblueh jblueh deleted the fix_race_conditions branch July 9, 2022 22:32
@jblueh jblueh mentioned this pull request Jul 3, 2023
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:fix PR: merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Hybrid parallel AD regression tests fail sometimes

5 participants

@jblueh @TobiKattmann @pcarruscag @maxaehle