Skip to content

add nvhpc compiler flags to cmake #1743

Open
JorgeG94 wants to merge 16 commits intoNOAA-GFDL:mainfrom
JorgeG94:feat/nvhpc-options
Open

add nvhpc compiler flags to cmake #1743
JorgeG94 wants to merge 16 commits intoNOAA-GFDL:mainfrom
JorgeG94:feat/nvhpc-options

Conversation

@JorgeG94
Copy link
Copy Markdown

@JorgeG94 JorgeG94 commented Aug 8, 2025

Description
Add the necessary flags to compile FMS with NVHPC using the CMake build system

Fixes # (issue)

How Has This Been Tested?
CMake does not have the unit tests connected to the build system but building using the autoconf mode with NVHPC fails most of the tests. Which I believe is known.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

mosaic2/include
constants
astronomy/include
field_manager/
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When building with nvhpc it could not find parse.inc which is here

Copy link
Copy Markdown
Contributor

@rem1776 rem1776 left a comment

Choose a reason for hiding this comment

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

Thanks for putting in these changes! I have some other cmake changes (#1694 ) that will be going in soon so this might need an update after that is merged but based on what im seeing it shouldn't be anything major.

@JorgeG94
Copy link
Copy Markdown
Author

We should push up the minimum cmake version - before cmake 3.20 there is no NVHPC compiler it goes back to the PGI one for which no flags are defined. Is there a reason to keep the cmake version minimum at 3.12?

@JorgeG94
Copy link
Copy Markdown
Author

only these tests fail (with nvhpc 24.9)

The following tests FAILED:
          3 - test_axis_utils2 (Failed)
          5 - test_column_diagnostics (Failed)
          6 - test_coupler (Failed)
          9 - test_data_override2_mono (Failed)
         10 - test_data_override2_ongrid (Failed)
         11 - test_data_override2_scalar (Failed)
         19 - test_diag_manager2 (Failed)
         37 - test_zbounds_limits (Failed)
         42 - test_atmosphere_io (Failed)
         43 - test_bc_restart (Failed)
         44 - test_collective_io (Failed)
         45 - test_fms2_io (Failed)
         46 - test_global_att (Failed)
         47 - test_io_simple (Failed)
         48 - test_io_with_mask (Failed)
         52 - test_interpolator2 (Failed)
         54 - test_mosaic2 (Failed)
         56 - test_global_arrays (Failed)
         62 - test_mpp_clock_begin_end_id (Failed)
         63 - test_mpp_domains2 (Failed)
         73 - test_mpp_nesting (Failed)
         94 - test_random_numbers (Failed)
         95 - test_sat_vapor_pres (Failed)
         96 - test_string_utils (Failed)
         97 - test_time_interp2 (Failed)
         99 - test_topography (Failed)
Errors while running CTest

@rem1776
Copy link
Copy Markdown
Contributor

rem1776 commented Aug 26, 2025

We should push up the minimum cmake version - before cmake 3.20 there is no NVHPC compiler it goes back to the PGI one for which no flags are defined. Is there a reason to keep the cmake version minimum at 3.12?

No I don't think so, feel free to bump it up if needed.

@rem1776
Copy link
Copy Markdown
Contributor

rem1776 commented Jan 21, 2026

@JorgeG94 Just following up, do you plan on updating these changes? We need support for nvhpc so I will get this up to date and merged if not.

@JorgeG94
Copy link
Copy Markdown
Author

JorgeG94 commented Jan 21, 2026

@JorgeG94 Just following up, do you plan on updating these changes? We need support for nvhpc so I will get this up to date and merged if not.

Hot damn I had completely forgotten about this! I will update these changes!

@rem1776 I've merged in the changes and testing right now. Are there any other changes that I need to do? I think I addressed the initial oens

Copy link
Copy Markdown
Contributor

@rem1776 rem1776 left a comment

Choose a reason for hiding this comment

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

@JorgeG94 No I think this all looks good, thanks for getting it updated. It'll just need the whitespace change I commented on so we can get all the CI checks passing.

@JorgeG94
Copy link
Copy Markdown
Author

JorgeG94 commented Jan 21, 2026

nvhpc 25.9 and 25.5 fails with:

NVFORTRAN-S-0155-Illegal POINTER assignment - pointer target must be simply contiguous (/home/jorge/nci/projects/access-nri/FMS/exchange/xgrid.F90: 4084)
  0 inform,   0 warnings,   1 severes, 0 fatal for get_1_from_xgrid_repro

I didn't see this back in August but I can't really remember what version of nvhpc I used! so I am trying all the ones I have. Is there a target version you have in mind?

I've committed the whitespace change

Update, it comes from #1778 will try to fix

@JorgeG94
Copy link
Copy Markdown
Author

JorgeG94 commented Feb 1, 2026

it seems we need to update the cmake version that is used in the CI if this PR goes through, it failed because I am using 3.20 and I require 3.22 (to ensure nvhpc support)

@rem1776
Copy link
Copy Markdown
Contributor

rem1776 commented Feb 5, 2026

it seems we need to update the cmake version that is used in the CI if this PR goes through, it failed because I am using 3.20 and I require 3.22 (to ensure nvhpc support)

Sorry about that, I'm working on getting our image updated. Will let you know when the new image is in use on the main branch.

@JorgeG94
Copy link
Copy Markdown
Author

I am testing FMS with the AMDFlang compiler and I've had to do similar changes to the ones in this branch. Are we at a good point to think about merging this?

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.

2 participants