Skip to content

Fix omp reproducible #727

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
Open
JanStreffing wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix omp reproducible #727

JanStreffing wants to merge 8 commits into from

Conversation

@JanStreffing
Copy link
Collaborator

@JanStreffing JanStreffing commented Jun 25, 2025

Turning on the -DOPENMP_REPRODUCIBLE=ON flag results in many compiler errors with gcc 13 on levante.
Here are all the changes necessary to compile again.

@JanStreffing JanStreffing requested a review from dsidoren June 25, 2025 20:47
@JanStreffing JanStreffing self-assigned this Jun 25, 2025
@JanStreffing JanStreffing added the bug Something isn't working label Jun 25, 2025
@JanStreffing JanStreffing requested a review from suvarchal June 25, 2025 20:48
@JanStreffing
Copy link
Collaborator Author

JanStreffing commented Jun 25, 2025 •
edited
Loading

Currently running tests with 4 threads to see if we really are bit-reproducible.

@JanStreffing
Copy link
Collaborator Author

JanStreffing commented Jun 27, 2025

While this compiles with -DOPENMP_REPRODUCIBLE=ON it blows up in step 1. Even when only one thread is used:

118:  ___CHECK FOR BLOW UP___________ --> mstep=           1
118:   --STOP--> found eta_n become NaN or <-10.0, >10.0
118:  mype     =          118
118:  mstep    =            1
118:  node     =          912
118:  uln, nln    =            1          30
118:  glon,glat   =   -12.7394000000000        51.2200000000000
118:
118:  eta_n(n)    =    11.6688147357911
118:  d_eta(n)    =  -3.529929058722991E-002
118:
118:  zbar_3d_n   =    11.6688147357909        6.62991868667159        1.59102263755229       -8.48676946068632       -18.5645615589249       -28.6423536571635       -38.7201457554021       -48.7979378536407       -58.8757299518794       -68.9535220501180       -79.0313141483566       -89.1091062465952       -104.225794393953       -124.381378590430       -149.575858836027       -179.809235130743       -220.120403523697       -270.509364014890       -330.976116604322       -401.520661291992       -482.142998077901       -572.843126962048       -673.621047944434       -784.476761025059       -905.410266203922       -1036.42156348102       -1177.51065285636       -1328.67753432994       -1500.00000000000       -1810.00000000000       0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000  0.000000000000000E+000

Using -DOPENMP_REPRODUCIBLE=OFF we can run with and without OMP threads and get no crashes.

@JanStreffing JanStreffing marked this pull request as draft June 27, 2025 09:53
@JanStreffing JanStreffing added this to the FESOM 2.7 milestone Aug 19, 2025
@JanStreffing JanStreffing linked an issue Sep 2, 2025 that may be closed by this pull request
broken -DOPENMP_REPRODUCIBLE=ON #528
Open
@JanStreffing
Copy link
Collaborator Author

JanStreffing commented Sep 2, 2025

@dsidoren, could you have a look at this PR and issue?

@JanStreffing JanStreffing marked this pull request as ready for review November 3, 2025 05:59
@JanStreffing
Copy link
Collaborator Author

JanStreffing commented Nov 10, 2025

@dsidoren, does it run with or without being reproduciable?

@JanStreffing JanStreffing modified the milestones: FESOM 2.7, FESOM 2.7.1 Nov 17, 2025
@JanStreffing
Copy link
Collaborator Author

JanStreffing commented Nov 27, 2025

Relevant or redundant? 20fa8de
These changes fix warnings that the cray ftn compiler on aleph spit out.

@JanStreffing
Merge branch 'main' into fix_omp_reproducible
e33cace
@JanStreffing
Fix OpenMP warnings: correct privatization and reduction clauses
b81ac04 
- icb_step.F90: Change area_ib_tot from PRIVATE to REDUCTION(+:) for proper parallel accumulation (fixes ftn-7208 warnings at lines 574 and 798)
- oce_ale_pressure_bv.F90: Change f_min from PRIVATE to FIRSTPRIVATE to preserve initialization value (fixes ftn-7208 warning at line 3037)
- write_step_info.F90: Remove loc_deta from REDUCTION clause as its result is unused (fixes ftn-7276 warning at line 117)
@JanStreffing
Copy link
Collaborator Author

JanStreffing commented Nov 27, 2025

I added the fixes from the cray ftn warnings here as well. Perhaps it helps. At least it should not break anything.

@JanStreffing
Fix preprocessor directive nesting in ice_EVP.F90
0d8cd77 
Fix 'unterminated #else' error by correctly placing __openmp_reproducible
check inside the ENABLE_OPENACC=false branch. The nested #ifndef was
incorrectly placed inside the #else block, causing a preprocessor error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dsidoren dsidoren Awaiting requested review from dsidoren

@suvarchal suvarchal Awaiting requested review from suvarchal

At least 1 approving review is required to merge this pull request.

Assignees

@dsidoren dsidoren

@JanStreffing JanStreffing

Labels

bug Something isn't working

Projects

None yet

Milestone

FESOM 2.7.1

Development

Successfully merging this pull request may close these issues.

broken -DOPENMP_REPRODUCIBLE=ON

3 participants

@JanStreffing @dsidoren