Skip to content

New implementation of differing module name#646

Merged
climbfuji merged 9 commits into
NCAR:developfrom
gold2718:source_module_ddt_fixes
Apr 22, 2025
Merged

New implementation of differing module name#646
climbfuji merged 9 commits into
NCAR:developfrom
gold2718:source_module_ddt_fixes

Conversation

@gold2718
Copy link
Copy Markdown
Collaborator

New implementation of differing module name

Implement a solution that allows a Fortran module name to differ from the filename.

User interface changes?: Yes
An optional module_name keyword is added to the ccpp-table-properties (MetadataTable) section that allows the user to specify a Fortran module name that is independent from the name of the enclosing file.

I feel this PR is a cleaner solution to #636 and should replace #637.

Fixes: #636

Testing:
test removed: NA
unit tests: PASS
system tests: PASS
manual testing: manually ran Fortran and unit tests

@gold2718 gold2718 added the bugfix Fix for issue with 'bug' label. label Feb 28, 2025
@gold2718 gold2718 added this to the capgen unification milestone Feb 28, 2025
@gold2718 gold2718 self-assigned this Feb 28, 2025
Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Apologies if I am not seeing it, but where in the updated tests is the new functionality (scheme name different from file name) tested? I see it for the DDT module name (not equal to file name), though.

@gold2718
Copy link
Copy Markdown
Collaborator Author

Apologies if I am not seeing it, but where in the updated tests is the new functionality (scheme name different from file name) tested? I see it for the DDT module name (not equal to file name), though.

Good catch! I started with the test mods from #637, however, I just added one (and fixed an error message that I found in the process).

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

I am withdrawing my approval for the moment solely based on the conversation in #637 (comment)

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

@dustinswales, the other CCPP framework developers, and I discussed that we should fix the CCPP physics so that no host model DDTs need to be passed to the physics instead of updating capgen. I did this in ufs-community/ufs-weather-model#2651 and PRs linked in the description. Therefore, I don't see any problems with this PR.

@gold2718
Copy link
Copy Markdown
Collaborator Author

I uploaded a change that allows schemes to use host model DDTs but I now have a conflict in the var_compatibility test that I have to resolve.

Copy link
Copy Markdown
Collaborator

@peverwhee peverwhee left a comment

Choose a reason for hiding this comment

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

One question, but not enough to stop my approval!

Thanks @gold2718

Comment thread test/var_compatibility_test/run_test Outdated
Comment thread scripts/metadata_table.py Outdated
Comment thread scripts/ccpp_capgen.py
@gold2718 gold2718 force-pushed the source_module_ddt_fixes branch from c864640 to f4e57b1 Compare April 13, 2025 20:07
@gold2718 gold2718 requested a review from dustinswales April 13, 2025 20:15
Copy link
Copy Markdown
Member

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

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

@gold2718 Thanks for this cleaner approach!
I've rebased #640 on top of this and all tests pass.

@dustinswales
Copy link
Copy Markdown
Member

dustinswales commented Apr 22, 2025

@climbfuji @peverwhee @mkavulich
We can merge this one after @gold2718 pulls in #657 (review)

@climbfuji climbfuji merged commit c322375 into NCAR:develop Apr 22, 2025
19 checks passed
climbfuji added a commit that referenced this pull request May 22, 2025
## Summary
Allow for sub components of a DDT to be passed into the Group caps from
the Host cap .

## Description
This PR adds recursive searching for DDTs when creating the call strings
to the Group caps. This preserves the full reference in the call string.

Snippet from `test_host_ccpp_cap.F90` before the change to
ddt_library.py:
`sfc_up_sw=phys_state%sfc_up_sw(col_start:col_end)`

After, with full reference in call string:
`sfc_up_sw=phys_state%fluxSW%sfc_up_sw(col_start:col_end)`

User interface changes?: No

Fixes: #639

This is built on #646 

Testing:
  New testing added to pass subfields of DDT into scheme.

---------

Co-authored-by: Steve Goldhaber <stevenng@met.no>
Co-authored-by: Dom Heinzeller <dom.heinzeller@icloud.com>
@climbfuji climbfuji moved this to Done in capgen unification Aug 28, 2025
@gold2718 gold2718 deleted the source_module_ddt_fixes branch December 29, 2025 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix for issue with 'bug' label.

Projects

Development

Successfully merging this pull request may close these issues.

4 participants