Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update master from dtc/develop 2020/03/17 #268

Merged

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Mar 17, 2020

@climbfuji
Copy link
Collaborator Author

Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

This looks okay but in investigating the change, I came across this comment in ccpp_prebuild:

each variable can be passed to a subroutine only once, there can be no overlapping/conflicting

Does this mean there should be a check that no subroutine has two dummy arguments with the same standard_name property? I do not think I have such a check now. The only use cases I can think of are to do unit conversion or kind conversion but the framework should handle both of those.

@climbfuji
Copy link
Collaborator Author

This looks okay but in investigating the change, I came across this comment in ccpp_prebuild:

each variable can be passed to a subroutine only once, there can be no overlapping/conflicting

Does this mean there should be a check that no subroutine has two dummy arguments with the same standard_name property? I do not think I have such a check now. The only use cases I can think of are to do unit conversion or kind conversion but the framework should handle both of those.

Yes, that is correct - a standard_name can only appear once in the metadata for a specific subroutine. This would be a dangerous feature to have - what happens if both variables were modified inside in a different way?

@gold2718
Copy link
Collaborator

This would be a dangerous feature to have - what happens if both variables were modified inside in a different way?

That is obviously a bad thing but what if one was intent(in) and the other intent(out)? My point is not that we should not have the check but that we should make sure we remove any need to have something like this. Based on my limited imagination of the weird things scheme writers do, I think we have the bases covered but an explicit check is needed (which I will make sure is in the capgen version).

@gold2718
Copy link
Collaborator

Update: That check is already active because a scheme's variables are in a VarDictionary object and add_variable is only ever called with exists_ok=False (the default value).

As Emily Litella said, "Never mind"

Copy link
Contributor

@llpcarson llpcarson left a comment

Choose a reason for hiding this comment

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

approved

@climbfuji climbfuji merged commit d32b965 into NCAR:master Mar 18, 2020
@climbfuji climbfuji deleted the update_ncar_master_from_dtc_develop_20200317 branch June 27, 2022 03:10
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.

3 participants