Skip to content

Fix omp reproducible#727

Open
JanStreffing wants to merge 10 commits intomainfrom
fix_omp_reproducible
Open

Fix omp reproducible#727
JanStreffing wants to merge 10 commits intomainfrom
fix_omp_reproducible

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.
@JanStreffing
Merge branch 'main' into fix_omp_reproducible
831735d
@JanStreffing
save wip
ea47b28
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

2 participants

@JanStreffing @dsidoren