Address compliance issues towards NCO HPC implementation standards for AQMv7 operational implementation

[rmontuoro]

This PR includes changes to allow AQMv7 to run on WCOSS2 after the code is built using the -check all compiler flag.

This is a requirement for the operational implementation of AQMv7.

NOTE: The proposed changes are limited to the AQMv7 operational configuration. Additional changes may be required for full compliance

[JongilHan66]

I am not familiar with "pointer" declaration in Fortran. Why are some variables declared as "pointer" while others are not?

[rmontuoro]

I am not familiar with "pointer" declaration in Fortran. Why are some variables declared as "pointer" while others are not?

The pointer attribute for dummy arguments is used when unallocated actual pointer arguments are passed to subroutines.
For example, the pointer array dtend is only allocated if Model%ldiag3d is .true. https://github.com/NOAA-EMC/fv3atm/blob/23bbfcd6013447e248be5e64e834b5b98dc075cf/ccpp/data/GFS_typedefs.F90#L6908-L6909.

[JongilHan66]

I am not familiar with "pointer" declaration in Fortran. Why are some variables declared as "pointer" while others are not?

The pointer attribute for dummy arguments is used when unallocated actual pointer arguments are passed to subroutines. For example, the pointer array dtend is only allocated if Model%ldiag3d is .true. https://github.com/NOAA-EMC/fv3atm/blob/23bbfcd6013447e248be5e64e834b5b98dc075cf/ccpp/data/GFS_typedefs.F90#L6908-L6909.

Thanks!!

[BrianCurtis-NOAA]

@JongilHan66 is there a reason this was closed?

[RuiyuSun]

@JongilHan66 is there a reason this was closed?

I believe it was closed by a mistake. @JongilHan66 @BrianCurtis-NOAA

[climbfuji]

This PR needs a thorough discussion, there are many changes that are not really CCPP compliant ...

[climbfuji]

@rmontuoro Why are some conditionally allocated arrays declared as "pointer" while others are declared as "allocatable"? If I remember correctly, there is no difference between how these arrays are declared in the UFS. Does every variable that gets modified to be "pointer" or "allocatable" in this PR have the correct active attribute in the metadata?

[JongilHan66]

@JongilHan66 is there a reason this was closed?

I believe it was closed by a mistake. @JongilHan66 @BrianCurtis-NOAA

Sorry, it was a mistake.

[yangfanglin]

@rmontuoro Raffaele, More discussion among the parties involved including NCO is required to make UFS/CCPP code comply with both CCPP and NCO EE2 standards. It will take some time to coordinate and find a solution. Committing your modifications back to CCPP authoritative repo is questionable at the moment. Since the submission of AQMv7 code to NCO is imminent, I'd recommend we use a branch or fork of the CCPP with your modifications included for AQMv7 code handoff. Of course, this may not be the best solution. Other suggestions are welcome (@climbfuji @grantfirl @dustinswales @ligiabernardet @junwang-noaa )

[BrianCurtis-NOAA]

I'd recommend we use a branch or fork of the CCPP with your modifications included for AQMv7 code handoff.

@yangfanglin This PR is requesting a merge into ufs-community:production/AQM.v7 is this sufficient?

[climbfuji]

@BrianCurtis-NOAA I'll reopen the PR, it got closed yesterday by mistake.

[junwang-noaa]

As @rmontuoro The code changes are limited to AQMv7 configuration, it might be good to list some sample warning message for further discussion. From code commit point of view, I think it is safe to commit to production/AQMv7 branch, which has a revision from Jan 2023. I was told HAFSv2 needs to freeze the code in Jan 2024, so we may need to discuss a solution to fix the warning message with "-check all" compile option in the develop branch.

[climbfuji]

@grantfirl @mkavulich @dustinswales I noticed the CI tests are failing, but at first glance this doesn't look like being related to the changes in this PR?

[rmontuoro]

@yangfanglin, @climbfuji - This PR is intended to introduce changes to the production/AQM.v7 branch only. Such changes are required for EE2 compliance and are limited to the AQMv7 configuration only. I agree and look forward to a conversation with CCPP developers to find a way forward for future operational implementations including CCPP.

[yangfanglin]

@rmontuoro, agreed with the temporary solution for AQMv7. Ligia has set up a meeting to discuss for a future solution. I have asked for a few EMC developers and Steven Earle to be included in the discussion.

[gold2718]

I would also like to see the warning messages that prompted this approach.
I am having trouble creating a test that reproduces this problem so that we can explore the solution space.

[grantfirl]

@rmontuoro Could you start an issue in this repository that details exactly the problem that is showing up with -check all on WCOSS2?

I agree with the temporary solution of isolating these changes to the production/AQM.v7 branch for now.

[mkavulich]

@grantfirl @mkavulich @dustinswales I noticed the CI tests are failing, but at first glance this doesn't look like being related to the changes in this PR?

That's correct, it appears this failure occurs in the main branch as well:

https://github.com/ufs-community/ccpp-physics/actions/runs/6935248044/job/18865022199

[grantfirl]

@grantfirl @mkavulich @dustinswales I noticed the CI tests are failing, but at first glance this doesn't look like being related to the changes in this PR?

That's correct, it appears this failure occurs in the main branch as well:

https://github.com/ufs-community/ccpp-physics/actions/runs/6935248044/job/18865022199

Yes, this is a known failure. I have started a branch to fix this. It looks like we can probably use the new (in 3.7) str.isascii() in the check_encoding.py script to fix this as long as GitHub CI can use python 3.7 or later.

[grantfirl]

@mkavulich @climbfuji FYI: #133

This appears to fix the CI failure, although I need to test it by injecting a non-ASCII character into a scheme and see if the script successfully detects it.

[JianpingHuang-NOAA]

@yangfanglin Here is the error message when the ufs-weateher-model was compiled with the flag of " -check noarg_temp_created" in the CCPP physics that Brian created.

https://ftp.emc.ncep.noaa.gov/mmb/aq/aqm.v7/errfile

Jianping

[grantfirl]

@yangfanglin Here is the error message when the ufs-weateher-model was compiled with the flag of " -check noarg_temp_created" in the CCPP physics that Brian created.

https://ftp.emc.ncep.noaa.gov/mmb/aq/aqm.v7/errfile

Jianping

@JianpingHuang-NOAA I don't see anything pointing to a CCPP problem in that logfile, although if I try to reproduce an error on Hera using ufs-community/ufs-weather-model#2007, NOAA-EMC/fv3atm#725, and the top of the ufs-community:production/AQM.v7 branch, I get the following runtime error:
err.txt

[yangfanglin]

@JianpingHuang-NOAA I thought there are also runtime Warnings in the forecast log file related to CCPP Raffaele (@rmontuoro ) pointed out. Can you find them and copy a few lines to this PR ?

[rmontuoro]

@yangfanglin - Issue #134 includes one of the runtime error messages occurring in AQMv7.

[grantfirl]

@yangfanglin - Issue #134 includes one of the runtime error messages occurring in AQMv7.

Thanks. That is the same error that I can reproduce on Hera.

[climbfuji]

Approving based on today's conversation. For the record, we are going to work on a separate PR for develop that addresses NCO compliancy and CCPP requirements.

[grantfirl]

@rmontuoro @BrianCurtis-NOAA @dustinswales @JianpingHuang-NOAA @junwang-noaa @aerorahul @climbfuji @mkavulich @scrasmussen and I all met offline to discuss this PR. We all agreed that it is fine for this PR to go into the production/AQM.v7 branch and that we'll introduce PRs into fv3atm and ccpp-physics that should be able to pass the -check all compilation flag in the near future for the develop and ufs/dev branches, respectively.

[yangfanglin]

Sorry I missed the meeting due to a conflict. The solution you reached at the meeting makes a lot of sense.

[grantfirl]

@rmontuoro Let me know when you're ready to merge. If you have permissions, you can feel free to do so too.

[rmontuoro]

@rmontuoro Let me know when you're ready to merge. If you have permissions, you can feel free to do so too.

@grantfirl - we are ready to merge.

[climbfuji]

I'll do it.