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 OMP race conditions and some chkcsum calls #142

Merged
merged 7 commits into from
Mar 9, 2020
Merged

Conversation

alperaltuntas
Copy link
Member

  • Fixes OMP race conditions in:
    • MOM.F90
    • MOM_diagnostics.F90
    • MOM_set_viscosity.F90
  • Comments out an OMP parallel do block in MOM_tracer_advect.F90, because, advect_x and advect_y are not thread-safe when computing ad2d_x and ad2d_y diagnostics. TODO: fix this and uncomment OMP directive again.
  • Also commented out an OMP parallel do block in KPP module, similarly, due to a thread-unsafe subroutine.
  • Fixes CS%Debug = .true. cases by initializing initialize fluid entrainment arrays, and by ensuring chksum for drag_vel is called only if it is allocated.

After these changes, PET tests and tests with CS%debug passes.

@codecov-io
Copy link

Codecov Report

Merging #142 into dev/ncar will increase coverage by 4.11%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/ncar     #142      +/-   ##
============================================
+ Coverage     41.13%   45.25%   +4.11%     
============================================
  Files           213      213              
  Lines         63251    63253       +2     
============================================
+ Hits          26020    28624    +2604     
+ Misses        37231    34629    -2602
Impacted Files Coverage Δ
src/diagnostics/MOM_diagnostics.F90 87.44% <ø> (+0.34%) ⬆️
src/core/MOM.F90 69.39% <ø> (+0.4%) ⬆️
src/parameterizations/vertical/MOM_CVMix_KPP.F90 0.84% <ø> (ø) ⬆️
src/tracer/MOM_tracer_advect.F90 69.03% <ø> (ø) ⬆️
...c/parameterizations/vertical/MOM_set_viscosity.F90 70.57% <ø> (+12.35%) ⬆️
...parameterizations/vertical/MOM_diabatic_driver.F90 58.68% <0%> (+3.32%) ⬆️
src/parameterizations/lateral/MOM_MEKE.F90 68.73% <100%> (+65.86%) ⬆️
src/core/MOM_barotropic.F90 71.37% <0%> (-0.8%) ⬇️
src/initialization/MOM_state_initialization.F90 31.53% <0%> (+0.21%) ⬆️
src/framework/MOM_file_parser.F90 67.58% <0%> (+0.25%) ⬆️
... and 41 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46b1f34...f832925. Read the comment docs.

@npotts
Copy link

npotts commented Mar 6, 2020

Codecov Report

Merging #142 into dev/ncar will increase coverage by 4.11%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/ncar     #142      +/-   ##
============================================
+ Coverage     41.13%   45.25%   +4.11%     
============================================
  Files           213      213              
  Lines         63251    63253       +2     
============================================
+ Hits          26020    28624    +2604     
+ Misses        37231    34629    -2602     
Impacted Files Coverage Δ
src/core/MOM_barotropic.F90 71.37% <0.00%> (-0.80%) ⬇️
src/initialization/MOM_state_initialization.F90 31.53% <0.00%> (+0.21%) ⬆️
src/framework/MOM_file_parser.F90 67.58% <0.00%> (+0.25%) ⬆️
src/parameterizations/lateral/MOM_hor_visc.F90 61.77% <0.00%> (+0.28%) ⬆️
src/framework/MOM_domains.F90 46.68% <0.00%> (+0.33%) ⬆️
src/diagnostics/MOM_diagnostics.F90 87.44% <0.00%> (+0.34%) ⬆️
src/core/MOM.F90 69.39% <0.00%> (+0.40%) ⬆️
src/diagnostics/MOM_sum_output.F90 60.66% <0.00%> (+0.45%) ⬆️
src/framework/MOM_checksums.F90 77.90% <0.00%> (+0.47%) ⬆️
src/tracer/MOM_tracer_flow_control.F90 53.45% <0.00%> (+0.72%) ⬆️
... and 34 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46b1f34...f832925. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants