-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add support (and tests) for optional arguments in ccpp_prebuild #552
Changes from 14 commits
cd5eae8
28b15cc
853f43d
6556d35
5e4bae1
363e8e5
f4d4d7f
413e9ec
61b0c61
8709a48
f9976d6
01e2d8d
8138e22
a6e9f04
f2b9a55
259a494
fba4ce0
0e00c17
7d2af1d
c84c9f0
bb20702
d769bb8
b8b754c
87f9323
caae916
788d251
ba27abd
f73bbe8
3839c63
f1fde4a
c298399
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ def __init__(self, **kwargs): | |
self._kind = None | ||
self._intent = None | ||
self._active = None | ||
self._optional = None | ||
self._pointer = False | ||
self._target = None | ||
self._actions = { 'in' : None, 'out' : None } | ||
for key, value in kwargs.items(): | ||
|
@@ -134,6 +136,30 @@ def active(self, value): | |
raise ValueError('Invalid value {0} for variable property active, must be a string'.format(value)) | ||
self._active = value | ||
|
||
@property | ||
def optional(self): | ||
'''Get the optional attribute of the variable.''' | ||
return self._optional | ||
|
||
@optional.setter | ||
def optional(self, value): | ||
if not isinstance(value, str): | ||
raise ValueError('Invalid value {0} for variable property optional, must be a string'.format(value)) | ||
self._optional = value | ||
|
||
# Pointer is not set by parsing metadata attributes, but by mkstatic. | ||
# This is a quick and dirty solution! | ||
@property | ||
def pointer(self): | ||
'''Get the pointer attribute of the variable.''' | ||
return self._pointer | ||
|
||
@pointer.setter | ||
def pointer(self, value): | ||
if not isinstance(value, bool): | ||
raise ValueError('Invalid value {0} for variable property pointer, must be a logical'.format(value)) | ||
self._pointer = value | ||
|
||
@property | ||
def target(self): | ||
'''Get the target of the variable.''' | ||
|
@@ -251,7 +277,10 @@ def dimstring_local_names(self, metadata, assume_shape = False): | |
dim.lower(), self.standard_name)) | ||
dim1 = metadata[dim.lower()][0].local_name | ||
if assume_shape: | ||
# DH* TEST w/o lower dim? | ||
dimstring.append('{}:'.format(dim0)) | ||
#dimstring.append(':') | ||
# *DH | ||
else: | ||
dimstring.append('{}:{}'.format(dim0, dim1)) | ||
return '({})'.format(','.join(dimstring)) | ||
|
@@ -270,62 +299,128 @@ def print_def_intent(self, metadata): | |
dictionary to resolve lower bounds for array dimensions.''' | ||
# Resolve dimensisons to local names using undefined upper bounds (assumed shape) | ||
dimstring = self.dimstring_local_names(metadata, assume_shape = True) | ||
# It is an error for host model variables to have the optional attribute in the metadata | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need to check if hosts having the optional attribute in Capgen is an error. I don't recall adding this, but maybe it's in there somewhere. @gold2718 Do you know by chance? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see where you could have put it. Is it possible that the distinction is really about not allowing the I'm not sure where this leaves DDTs. For a host DDT, can an element have the Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is about the metadata only. The rule is quite simple (if I understood it correctly):
prebuild checks for 1-5 in its own layer after receiving the metadata from capgen. This is because we only harvest information on host metadata and scheme metadata separately (except for passing known host DDTs) and do not rely on the metadata comparison host <--> model from capgen. I will note that at this point, prebuild spits out a warning for 6 if it finds a variable with optional being true but no non-default active attribute in the model. I used this to clean up the metadata, knowing that the current ccpp physics are tied to the GFS data structures used by UFS, SCM, NEPTUNE. Regarding your pointer question. That gets tricky. The only way out I can see here is that if a host ddt has an active attribute other than the default true, all it's members (if declared explicitly in the metadata) need that to have active to, with the allocation condition being that of the DDT (if the member is always allocated when the DDT is allocated) OR a Fortran logical && of the DDTs allocation condition and its own. We need to be explicit here and have both allocation conditions in the member var's active attribute instead of implicitly inheriting it from the DDT. That would greatly confuse the physics/model developers and users. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @climbfuji, I think your rules 1-6 plus thoughts on DDTs all make sense. We should make sure all of these are tested in capgen, do we need a new issue for that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that would be good. I'll work with @dustinswales on that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1,2,5, and 6 are applied In the Capgen implementation, with the exception of 3 and 4. I will add these soon, and revisit the DDTs. |
||
if self.optional == 'T': | ||
error_message = "This routine should only be called for host model variables" + \ | ||
" that cannot be optional, but got self.optional=T" | ||
raise Exception(error_message) | ||
# If the host variable is potentially unallocated, add optional and target to variable declaration | ||
elif not self.active == 'T': | ||
optional = ', optional, target' | ||
else: | ||
optional = '' | ||
# | ||
if self.type in STANDARD_VARIABLE_TYPES: | ||
if self.kind: | ||
str = "{s.type}({s._kind}), intent({s.intent}) :: {s.local_name}{dimstring}" | ||
str = "{s.type}({s._kind}), intent({s.intent}){optional} :: {s.local_name}{dimstring}" | ||
else: | ||
str = "{s.type}, intent({s.intent}) :: {s.local_name}{dimstring}" | ||
str = "{s.type}, intent({s.intent}){optional} :: {s.local_name}{dimstring}" | ||
else: | ||
if self.kind: | ||
error_message = "Generating variable definition statements for derived types with" + \ | ||
" kind attributes not implemented; variable: {0}".format(self.standard_name) | ||
raise Exception(error_message) | ||
else: | ||
str = "type({s.type}), intent({s.intent}) :: {s.local_name}{dimstring}" | ||
return str.format(s=self, dimstring=dimstring) | ||
str = "type({s.type}), intent({s.intent}){optional} :: {s.local_name}{dimstring}" | ||
return str.format(s=self, optional=optional, dimstring=dimstring) | ||
|
||
def print_def_local(self, metadata): | ||
'''Print the definition line for the variable, assuming it is a local variable.''' | ||
if self.type in STANDARD_VARIABLE_TYPES: | ||
if self.kind: | ||
if self.rank: | ||
str = "{s.type}({s._kind}), dimension{s.rank}, allocatable :: {s.local_name}" | ||
# It is an error for local variables to have the active attribute | ||
if not self.active == 'T': | ||
error_message = "This routine should only be called for local variables" + \ | ||
" that cannot have an active attribute other than the" +\ | ||
" default T, but got self.active=T" | ||
raise Exception(error_message) | ||
|
||
# If it is a pointer, everything is different! | ||
if self.pointer: | ||
if self.type in STANDARD_VARIABLE_TYPES: | ||
if self.kind: | ||
if self.rank: | ||
str = "{s.type}({s._kind}), dimension{s.rank}, pointer :: p" | ||
climbfuji marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else: | ||
str = "{s.type}({s._kind}), pointer :: p" | ||
else: | ||
str = "{s.type}({s._kind}) :: {s.local_name}" | ||
if self.rank: | ||
str = "{s.type}, dimension{s.rank}, pointer :: p" | ||
else: | ||
str = "{s.type}, pointer :: p" | ||
else: | ||
if self.rank: | ||
str = "{s.type}, dimension{s.rank}, allocatable :: {s.local_name}" | ||
if self.kind: | ||
error_message = "Generating variable definition statements for derived types with" + \ | ||
" kind attributes not implemented; variable: {0}".format(self.standard_name) | ||
raise Exception(error_message) | ||
else: | ||
str = "{s.type} :: {s.local_name}" | ||
if self.rank: | ||
str = "type({s.type}), dimension{s.rank}, pointer :: p" | ||
else: | ||
str = "type({s.type}), pointer :: p" | ||
return str.format(s=self) | ||
else: | ||
if self.kind: | ||
error_message = "Generating variable definition statements for derived types with" + \ | ||
" kind attributes not implemented; variable: {0}".format(self.standard_name) | ||
raise Exception(error_message) | ||
# If the host variable is potentially unallocated, the active attribute is | ||
# also set accordingly for the local variable; add target to variable declaration | ||
if self.optional == 'T': | ||
target = ', target' | ||
else: | ||
if self.rank: | ||
str = "type({s.type}), dimension{s.rank}, allocatable :: {s.local_name}" | ||
target = '' | ||
if self.type in STANDARD_VARIABLE_TYPES: | ||
if self.kind: | ||
if self.rank: | ||
str = "{s.type}({s._kind}), dimension{s.rank}, allocatable{target} :: {s.local_name}" | ||
else: | ||
str = "{s.type}({s._kind}){target} :: {s.local_name}" | ||
else: | ||
str = "type({s.type}) :: {s.local_name}" | ||
return str.format(s=self) | ||
if self.rank: | ||
str = "{s.type}, dimension{s.rank}, allocatable{target} :: {s.local_name}" | ||
else: | ||
str = "{s.type}{target} :: {s.local_name}" | ||
else: | ||
if self.kind: | ||
error_message = "Generating variable definition statements for derived types with" + \ | ||
" kind attributes not implemented; variable: {0}".format(self.standard_name) | ||
raise Exception(error_message) | ||
else: | ||
if self.rank: | ||
str = "type({s.type}), dimension{s.rank}, allocatable{target} :: {s.local_name}" | ||
else: | ||
str = "type({s.type}){target} :: {s.local_name}" | ||
return str.format(s=self, target=target) | ||
|
||
def print_debug(self): | ||
'''Print the data retrieval line for the variable.''' | ||
str='''Contents of {s} (* = mandatory for compatibility): | ||
standard_name = {s.standard_name} * | ||
long_name = {s.long_name} | ||
units = {s.units} * | ||
local_name = {s.local_name} | ||
type = {s.type} * | ||
dimensions = {s.dimensions} | ||
rank = {s.rank} * | ||
kind = {s.kind} * | ||
intent = {s.intent} | ||
active = {s.active} | ||
target = {s.target} | ||
container = {s.container} | ||
actions = {s.actions}''' | ||
# Scheme variables don't have the active attribute | ||
if 'SCHEME' in self.container: | ||
str='''Contents of {s} (* = mandatory for compatibility): | ||
standard_name = {s.standard_name} * | ||
long_name = {s.long_name} | ||
units = {s.units} * | ||
local_name = {s.local_name} | ||
type = {s.type} * | ||
dimensions = {s.dimensions} | ||
rank = {s.rank} * | ||
kind = {s.kind} * | ||
intent = {s.intent} | ||
optional = {s.optional} | ||
target = {s.target} | ||
container = {s.container} | ||
actions = {s.actions}''' | ||
# Host model variables don't have the optional attribute | ||
else: | ||
str='''Contents of {s} (* = mandatory for compatibility): | ||
standard_name = {s.standard_name} * | ||
long_name = {s.long_name} | ||
units = {s.units} * | ||
local_name = {s.local_name} | ||
type = {s.type} * | ||
dimensions = {s.dimensions} | ||
rank = {s.rank} * | ||
kind = {s.kind} * | ||
intent = {s.intent} | ||
active = {s.active} | ||
target = {s.target} | ||
container = {s.container} | ||
actions = {s.actions}''' | ||
return str.format(s=self) | ||
|
||
class CapsMakefile(object): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly for the optional property: