Add threading test to `capgen_test`; codee format `test/capgen_test/*.F90` by climbfuji · Pull Request #722 · NCAR/ccpp-framework

climbfuji · 2026-03-02T15:56:05Z

Add threading test to capgen_test. This is exercised with ctest without any further changes, and it passes. I added a comparison of all averages to the compare_data call, which compares the omp1 and omp2 test runs. This is commit 900069b. While testing this, @peverwhee and I uncovered a bug (uninitialized variable, see Add threading test to capgen_test; codee format test/capgen_test/*.F90 #722 (comment)) that I had to fix.
Update Codee to 2025.4.8 and simplify .github/workflows/fortran-formatting.yaml (there was an actual error using the workflow that needed to be fixed). This is commit a76f82c
Codee format test/capgen_test/*.F90. This is commit 24223ff

User interface changes?: No

Issues: Working toward #721

Testing: all pass
test removed: n/a
unit tests: n/a
system tests: existing capgen test now run with OMP_NUM_THREADS=1 and OMP_NUM_THREADS=2
manual testing: tested manually on my dev machine with spack-stack-2.1.0

…tting.yaml

dustinswales

Thanks for adding these features.
I like how extensible the ctest was in this case!

dustinswales · 2026-03-11T15:41:30Z

test/capgen_test/CMakeLists.txt

This is slick!

peverwhee · 2026-03-12T19:46:15Z

hi @climbfuji i'm working through this now, but thought I'd mention that I ran the unit tests on derecho and found that the new thread tests fail (answer differences) with ifx (but pass with ifort). Not sure if this is critical. If we determine that we'll proceed without fixing that now, I will make an issue.

climbfuji · 2026-03-12T19:52:09Z

@peverwhee That's interesting. I thought I had tested on my laptop with ifx. Let me go back and retest. If it's only on Derecho, then we'll want to file an issue and fix it later.

climbfuji · 2026-03-12T22:22:12Z

@peverwhee That's interesting. I thought I had tested on my laptop with ifx. Let me go back and retest. If it's only on Derecho, then we'll want to file an issue and fix it later.

I figured that I tested with gcc on my laptop earlier. I can reproduce your error with oneapi@2025.3.0. The results are identical between omp1 and omp2 with oneapi@2025.3.0, but what is concerning is that the values are vastly different between gfortran and ifx - not just a bit. I will need to investigate what is going on here.

peverwhee · 2026-03-12T22:31:38Z

@peverwhee That's interesting. I thought I had tested on my laptop with ifx. Let me go back and retest. If it's only on Derecho, then we'll want to file an issue and fix it later.

I figured that I tested with gcc on my laptop earlier. I can reproduce your error with oneapi@2025.3.0. The results are identical between omp1 and omp2 with oneapi@2025.3.0, but what is concerning is that the values are vastly different between gfortran and ifx - not just a bit. I will need to investigate what is going on here.

Thanks for the update, @climbfuji

If it helps, it also fails with oneapi@2025.2.1. Let me know if you want a second pair of eyes. Since we haven't extensively tested the generated caps with ifx, it could be something somewhat unrelated to threading that's popping up

climbfuji · 2026-03-12T22:33:34Z

@peverwhee That's interesting. I thought I had tested on my laptop with ifx. Let me go back and retest. If it's only on Derecho, then we'll want to file an issue and fix it later.

I figured that I tested with gcc on my laptop earlier. I can reproduce your error with oneapi@2025.3.0. The results are identical between omp1 and omp2 with oneapi@2025.3.0, but what is concerning is that the values are vastly different between gfortran and ifx - not just a bit. I will need to investigate what is going on here.

Thanks for the update, @climbfuji

If it helps, it also fails with oneapi@2025.2.1. Let me know if you want a second pair of eyes. Since we haven't extensively tested the generated caps with ifx, it could be something somewhat unrelated to threading that's popping up

I am fairly certain that it's something else. Let me dig a little more later, maybe it has to do with the flags, uninitialized data, ...

… to 1

climbfuji · 2026-03-13T13:42:17Z

@peverwhee Indeed, I found an uninitialized variable (cind). After setting it to a safe and reasonable value of 1, the results match for gcc@13 and oneapi@2025.3.0. Interestingly, the value now matches what ifx produced prior to the bug fix, i.e. ifx must have initialized cind to 1. Please try again on your end.

The bug fix is in commit 419a6c2

peverwhee

thanks @climbfuji ! tests pass for me with both ifx and gnu on derecho.

I do have a couple of codee questions.

peverwhee · 2026-03-13T19:56:29Z

test/capgen_test/temp_adjust.F90

+    real(kind=kind_phys), intent(in) :: temp_prev(:)
+    real(kind=kind_phys), intent(inout) :: temp_layer(foo)


I'm wondering if maybe we don't want to fully codee-ify some of the test code? It seems like we're removing some testing by changing this (REAL(kind_phys) -> real(kind=kind_phys)). Since both "REAL" upper-case and no "kind=" are still valid fortran (even if it doesn't comply with our coding style)

This is a good point. I believe Codee has constructs that we can add to the code to prevent formatting certain sections. I'll look it up. If it works, I'll rather use that then excluding formatting of the entire file.

See commit c221aaf

peverwhee · 2026-03-13T20:00:06Z

test/capgen_test/temp_adjust.F90

-    if (.not. module_level_config) then
-       ! do nothing
-       return
+    if (.not.module_level_config) then


This is a total nit-pick and not something i noticed in the original codee PR, but are we sure we don't want a white space after ".not."?

I am open to changing the formatting rules, but we should handle this in a separate issue/discussion?

peverwhee · 2026-03-13T20:01:08Z

test/capgen_test/temp_calc_adjust.F90

+  !> \section arg_table_temp_calc_adjust_register  Argument Table
+  !! \htmlinclude arg_table_temp_calc_adjust_register.html
+  !!
+  subroutine temp_calc_adjust_register(dim_inter, errmsg, errflg)


similar concern about this case change resulting in a decrease of testing for our fortran parser.

#722 (comment)

See commit c221aaf

peverwhee · 2026-03-13T20:06:06Z

test/capgen_test/test_host_mod.F90

+  !!
+  integer, parameter :: ncols = 10
+  integer, parameter :: pver = 5
+  integer, parameter :: pverp = 6


for completeness, i don't mind these case changes because the metadata has the upper-case version.

…st.F90 and test/capgen_test/temp_calc_adjust.F90 to test Fortran parser; try to prevent Codee from formatting these blocks

Add threading test to test/capgen_test

900069b

climbfuji requested review from a team and gold2718 as code owners March 2, 2026 15:56

Update codee to 2025.4.8 and simplify .github/workflows/fortran-forma…

a76f82c

…tting.yaml

climbfuji force-pushed the feature/capgen_threading_test branch from a9e77fa to a76f82c Compare March 2, 2026 17:04

Codee format test/capgen_test/*.F90

24223ff

climbfuji changed the title ~~Add threading test to test/capgen_test~~ Add threading test to capgen_test; codee format test/capgen_test/*.F90 Mar 2, 2026

dustinswales approved these changes Mar 11, 2026

View reviewed changes

test/capgen_test/CMakeLists.txt

Copy link

Member

dustinswales Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slick!

peverwhee reacted with hooray emoji

Fix bug in test/capgen_test/test_host_mod.F90: set uninitialized cind…

419a6c2

… to 1

climbfuji force-pushed the feature/capgen_threading_test branch from 3a38b76 to 419a6c2 Compare March 13, 2026 13:41

climbfuji self-assigned this Mar 13, 2026

climbfuji added the capgen bugs, requests, etc. that involve ccpp_capgen label Mar 13, 2026

peverwhee reviewed Mar 13, 2026

View reviewed changes

Revert formatting of certain constructs in test/capgen_test/temp_adju…

c221aaf

…st.F90 and test/capgen_test/temp_calc_adjust.F90 to test Fortran parser; try to prevent Codee from formatting these blocks

climbfuji requested a review from peverwhee March 13, 2026 23:47

		real(kind=kind_phys), intent(in) :: temp_prev(:)
		real(kind=kind_phys), intent(inout) :: temp_layer(foo)

Conversation

climbfuji commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dustinswales left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peverwhee commented Mar 12, 2026

Uh oh!

climbfuji commented Mar 12, 2026

Uh oh!

climbfuji commented Mar 12, 2026

Uh oh!

peverwhee commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

climbfuji commented Mar 12, 2026

Uh oh!

climbfuji commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peverwhee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

climbfuji commented Mar 2, 2026 •

edited

Loading

peverwhee commented Mar 12, 2026 •

edited

Loading

climbfuji commented Mar 13, 2026 •

edited

Loading