-
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
DRAFT TO DISCUSS: Add support and testing for equivalent units #571
base: main
Are you sure you want to change the base?
Conversation
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.
That was quick!
This looks good to me.
@@ -176,3 +176,14 @@ def W_m_minus_2__to__erg_cm_minus_2_s_minus_1(): | |||
def erg_cm_minus_2_s_minus_1__to__W_m_minus_2(): | |||
"""Convert erg per square centimeter and second to Watt per square meter""" | |||
return '1.0E-3{kind}*{var}' | |||
|
|||
################## |
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.
I don't know what this is called in the DSM-5 but I really want a couple of more hash marks here :)
I know where to get some cheap :)
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.
This is not going to work with prebuild (at least the way the PR is written now). The unit conversions above return
def erg_cm_minus_2_s_minus_1__to__W_m_minus_2():
"""Convert erg per square centimeter and second to Watt per square meter"""
return '1.0E-3{kind}*{var}'
in other words, they contain the input variable in the result string. I think you need to make similar changes to the ccpp_prebuild logic (if None
then skip the unit conversion altogether), or you may get an error in the Python code, or in the auto-generated Fortran code.
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.
Yeah, I didn't add this to prebuild yet.
If we agree on this for Capgen, then I could add it in to Prebuild as part of this PR.
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.
@gold2718 Nothing is cheap in Norway, not even hashes from what I hear :)
And now I know what DSM-5 is.
(I will add some more hashes in for prettiness)
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.
Nothing is cheap in Norway, not even hashes from what I hear :)
That's what a harrytur* is for.
*Harrytur: Norwegian slang for driving to Sweden to buy stuff that's cheaper there.
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.
@dustinswales The easiest way (no changes at all in prebuild) would be to return just the input variable in the conversion string ({var}
) for equivalent units, not None
. If a compiler finds foo=foo
in the fortran code, it is very likely going to optimize out that line anyway.
@@ -176,3 +176,14 @@ def W_m_minus_2__to__erg_cm_minus_2_s_minus_1(): | |||
def erg_cm_minus_2_s_minus_1__to__W_m_minus_2(): | |||
"""Convert erg per square centimeter and second to Watt per square meter""" | |||
return '1.0E-3{kind}*{var}' | |||
|
|||
################## |
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.
This is not going to work with prebuild (at least the way the PR is written now). The unit conversions above return
def erg_cm_minus_2_s_minus_1__to__W_m_minus_2():
"""Convert erg per square centimeter and second to Watt per square meter"""
return '1.0E-3{kind}*{var}'
in other words, they contain the input variable in the result string. I think you need to make similar changes to the ccpp_prebuild logic (if None
then skip the unit conversion altogether), or you may get an error in the Python code, or in the auto-generated Fortran code.
################## | ||
# Equivalent units # | ||
################## | ||
def m2_s_minus_2__to__J_kg_minus_1(): |
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.
Just checking, does a unit string of m2 s-1
in the metadata files correspond to m2_s_minus_2
or m_2_s_minus_2
or m_plus_2_s_minus_2
?
[ 50 character, one line summary ]
[ 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. ]
User interface changes?: [ No/Yes ]
[ If yes, describe what changed, and steps taken to ensure backward compatibilty ]
Fixes: [Github issue #s] And brief description of each issue.
Testing:
test removed:
unit tests:
system tests:
manual testing: