Fix local name collisions between group cap dummy arguments and locals#771
Open
jimmielin wants to merge 1 commit into
Open
Fix local name collisions between group cap dummy arguments and locals#771jimmielin wants to merge 1 commit into
jimmielin wants to merge 1 commit into
Conversation
Two schemes in the same suite group using the same Fortran local name for different standard names generated an uncompilable group cap: a constituent dummy argument keeps the local name of the scheme that first requested it, while a scheme-supplied interstitial declares a group local with its own scheme local name. The Group call list and the Group dictionary each enforced local name uniqueness internally (NCAR#631) but never against each other, so the group subroutine declared the same name twice. The same collision occurred between a host variable dummy argument and an interstitial local. Fixes: - After a Group's analysis, rename any group-local variable whose local name matches a dummy argument (call list) or suite module variable (Group.ensure_unique_local_names). Renaming group locals is safe because generated code resolves variables by standard name at write time and locals are invisible outside the group subroutine. - VarDictionary.new_internal_variable_name accepts a reserved collection of additional names to avoid and compares candidate names case insensitively (local name use is case insensitive). - Add VarDictionary.known_local_names accessor. - Fix a latent KeyError in Group.write when a duplicate local name was first registered as a scalar or optional variable rather than an allocatable. Test: the advection test gains cld_shadow, a scheme reusing the cloud ice constituent local name (cld_ice_array) and the host horizontal dimension local name (ncols) for its own interstitials; the generated group cap must rename its locals for the test to compile and run. Seen in CAM-SIMA cam7: nucleate_ice_ccpp's constituent 'ni' collided with the gravity wave drag schemes' interstitial 'ni' (worked around in atmospheric_physics by renaming the scheme argument). Assisted-by: claude-fable:5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[ 50 character, one line summary ]
Fix local name collisions in generated group caps
[ Description of the changes in this commit. It should be enough
information for someone not following this development to understand.
Lines should be wrapped at about 72 characters. ]
Two schemes in the same suite group may use the same local name for
different quantities (i.e., different standard names).
capgen declared both in the group subroutine scope,
e.g. a constituent dummy argument (which keeps the local name of the first
requesting scheme)
alongside a group local for a scheme-supplied interstitial, or
a host variable dummy alongside such a local.
The call list and group dictionary each enforce internal uniqueness
(#631) but were never checked against each other, so capgen silently
generated uncompilable code (duplicate declarations).
After group analysis, Group.ensure_unique_local_names() now renames
any group-local variable whose name matches a dummy argument or suite
module variable. Renaming locals is safe: generated code resolves
variables by standard name at write time, and group locals are not
visible outside the group subroutine.
User interface changes?: No - bug fix only
Fixes: Issue #772 . Reproducer in PR. See
cld_ice_arrayandncolswhich now has two meanings.Testing:
test removed: n/a
unit tests: all PASS.
system tests: all PASS. see improved advection_test
manual testing: in CAM-SIMA
AI disclosure: claude-fable:5