Add unit conversion from degrees to radians, address isssue #309 #311

climbfuji · 2020-07-09T15:28:29Z

This PR contains the following changes:

scripts/conversion_tools/unit_conversion.py: add conversion from degrees to radians and vice versa, address issue Follow-up changes for PR https://github.com/NCAR/ccpp-framework/pull/307 #309
scripts/ccpp_prebuild.py, scripts/mkcap.py: address issue Follow-up changes for PR https://github.com/NCAR/ccpp-framework/pull/307 #309 (use correct Python syntax to test if a variable is a list or a dictionary)
a bugfix from @mzhangw for metadata2html.py (fix a bug to avoid reading keys in metadata2html.py #312)

climbfuji · 2020-07-09T15:52:23Z

@gold2718 @davegill @cacraigucar @grantfirl here is an interesting question - what are the official units to use for radians and degrees? I googled around to get some idea on what is used most. Some websites say radian and degree (singular, as for kg - you don't say "100 kilograms", but "100 kilogram"), but most websites use radians and degrees (plural; this is also how the units are used currently in FV3 and ccpp-physics).

gold2718 · 2020-07-09T16:51:06Z

Glad you asked. For radians, it is radian.
For latitude in degrees, it is degrees_north.
For longitude, in degrees, it is degrees_east.
Area should be steradian or radian2

Note these are all from UDUNITS which also accepts things like degree_north, degreeN, degree_N, degreesN, and degrees_N.

climbfuji · 2020-07-09T17:03:20Z

Glad you asked. For radians, it is radian.
For latitude in degrees, it is degrees_north.
For longitude, in degrees, it is degrees_east.
Area should be steradian or radian2

Note these are all from UDUNITS which also accepts things like degree_north, degreeN, degree_N, degreesN, and degrees_N.

Now I regret that I asked ;-) that's more complicated than what I wanted to hear.

…nary

…ees to radians and vice versa

climbfuji · 2020-07-09T18:30:19Z

Ok, for the moment I will implement "radian" to "degrees" (for variables that are not latitude or longitude), "degrees_north" and "degrees_east".

gold2718 · 2020-07-09T18:48:49Z

Do we have to accept everything for the CCPP? How about just radian, degrees_east, and degrees_north?

climbfuji · 2020-07-09T18:59:34Z

Do we have to accept everything for the CCPP? How about just radian, degrees_east, and degrees_north?

I would be ok with limiting it to those three, but we also need to have just "degree" if you think about variables such as the angle between the wind normal component on an MPAS edge and the meridional component, for example.

climbfuji · 2020-07-10T13:52:51Z

Associated PRs:
NOAA-EMC/fv3atm#140
NCAR/ccpp-physics#470
#311
ufs-community/ufs-weather-model#162

cacraigucar · 2020-07-10T14:08:14Z

I'd recommend we be consistent with the use or non-use of "s" for degree(s) and radian(s). Either "degree_east", "degree_north", "degree" and "radian" or "degrees_east", "degrees_north", "degrees" and "radians".

Since we are choosing specifically for CCPP, I personally like the plural better, but feel more strongly about there being consistency than I do on which flavor.

climbfuji · 2020-07-10T14:20:21Z

I'd recommend we be consistent with the use or non-use of "s" for degree(s) and radian(s). Either "degree_east", "degree_north", "degree" and "radian" or "degrees_east", "degrees_north", "degrees" and "radians".

Since we are choosing specifically for CCPP, I personally like the plural better, but feel more strongly about there being consistency than I do on which flavor.

That was my feeling, too - use plural for both. But as @gold2718 said udunits as a de-facto-standard (?) has radian singular and degrees plural. It's fine if we depart from that, but we all need to agree on this.

gold2718

Looks okay. Comment for future consideration.

gold2718 · 2020-07-10T21:28:10Z

scripts/conversion_tools/unit_conversion.py

+def radian__to__degrees():
+    """Convert radian to degrees"""
+    return '57.295779513{kind}*{var}'
+
+def degrees__to__radian():
+    """Convert degrees to radian"""
+    return '{var}/57.295779513{kind}'


While correct, this will eventually run afoul of efforts to standardize constants.
We should revisit (e.g., via pi / 180.0{kind} and its inverse) this then (i.e., when a universal value for pi is available to CCPP modules).

Yes, I agree. I was wondering if I should plug in the value of math.pi and then "calculate" 57.x as 180.0/pi, but that seemed to be unnecessary to me. Rather implement it correctly once pi is defined and available as a Fortran constant through the CCPP framework. Then one can set pi = 1 and the world suddenly looks a lot more familiar ;-)

We should definitely add that to the physical constants dictionary. What good would it be if there was no version with c == ℏ == 1?
c == ℏ == π == 1 is only for advanced users.

grantfirl · 2020-07-10T21:33:02Z

I'd recommend we be consistent with the use or non-use of "s" for degree(s) and radian(s). Either "degree_east", "degree_north", "degree" and "radian" or "degrees_east", "degrees_north", "degrees" and "radians".
Since we are choosing specifically for CCPP, I personally like the plural better, but feel more strongly about there being consistency than I do on which flavor.

That was my feeling, too - use plural for both. But as @gold2718 said udunits as a de-facto-standard (?) has radian singular and degrees plural. It's fine if we depart from that, but we all need to agree on this.

FWIW, in this case, I don't agree with the standard (plural just sounds better to the ear), but I think we should adhere to standards where possible, so my vote is to follow udunits.

climbfuji · 2020-07-10T21:37:18Z

I'd recommend we be consistent with the use or non-use of "s" for degree(s) and radian(s). Either "degree_east", "degree_north", "degree" and "radian" or "degrees_east", "degrees_north", "degrees" and "radians".
Since we are choosing specifically for CCPP, I personally like the plural better, but feel more strongly about there being consistency than I do on which flavor.

That was my feeling, too - use plural for both. But as @gold2718 said udunits as a de-facto-standard (?) has radian singular and degrees plural. It's fine if we depart from that, but we all need to agree on this.

FWIW, in this case, I don't agree with the standard (plural just sounds better to the ear), but I think we should adhere to standards where possible, so my vote is to follow udunits.

Thanks. We need to decide early next week, because this PR is next or second in row in the commit queue.

grantfirl · 2020-07-10T21:37:27Z

I'd recommend we be consistent with the use or non-use of "s" for degree(s) and radian(s). Either "degree_east", "degree_north", "degree" and "radian" or "degrees_east", "degrees_north", "degrees" and "radians".
Since we are choosing specifically for CCPP, I personally like the plural better, but feel more strongly about there being consistency than I do on which flavor.

That was my feeling, too - use plural for both. But as @gold2718 said udunits as a de-facto-standard (?) has radian singular and degrees plural. It's fine if we depart from that, but we all need to agree on this.

FWIW, in this case, I don't agree with the standard (plural just sounds better to the ear), but I think we should adhere to standards where possible, so my vote is to follow udunits.

I just realized that udunits is inconsistent with plurality too. Sigh. If they accept the singular degree, though, I vote for:
degree, degree_north, degree_east, radian.

gold2718 · 2020-07-10T23:19:51Z

UDUNITS accepts degree, degreeN, and degree_N (plus E for N).
There is no plural for radian

climbfuji · 2020-07-13T13:41:31Z

So, is the conclusion to go for "degree", "degree_north", "degree_east", and "radian"? If so, I'll update the PRs and unit conversion entries, we are next in line to commit.

Thanks!

grantfirl · 2020-07-13T16:40:29Z

@climbfuji Yes, I think that is the consensus.

gold2718 · 2020-07-13T16:44:19Z

But those are not in UDUNITS (except for degree and radian).

grantfirl · 2020-07-13T17:00:48Z

degree_north and degree_east look like they are in this udunits list as aliases: https://www.unidata.ucar.edu/software/udunits/udunits-current/udunits/udunits2-common.xml

Is that good enough? @gold2718 Where are you looking for udunits inclusion?

gold2718 · 2020-07-13T17:15:06Z

Yow, it looks like my version is out of date. I am okay with this selection as an 'official' CCPP list.

climbfuji · 2020-07-13T17:20:52Z

Ok, thanks, will proceed with this selection!

…

On Jul 13, 2020, at 11:15 AM, goldy ***@***.***> wrote: Yow, it looks like my version is out of date. I am okay with this selection as an 'official' CCPP list. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#311 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RKEB4BX2JCA4Y64C2LR3M6KTANCNFSM4OVW5SCQ>.

…ramework into add_degrees_to_radians_conversion

gold2718

Looks good.

climbfuji added 2 commits July 9, 2020 12:28

Use correct Python syntax to test if a variable is a list or a dictio…

a7b1f4b

…nary

scripts/conversion_tools/unit_conversion.py: add conversion from degr…

f375880

…ees to radians and vice versa

climbfuji force-pushed the add_degrees_to_radians_conversion branch from 405e3c2 to f375880 Compare July 9, 2020 18:28

DusanJovic-NOAA mentioned this pull request Jul 10, 2020

Allow multiple regtest ecflow suites to run on the same server ufs-community/ufs-weather-model#162

Merged

climbfuji marked this pull request as ready for review July 10, 2020 13:52

climbfuji requested review from gold2718, grantfirl and llpcarson as code owners July 10, 2020 13:52

This was referenced Jul 10, 2020

Cleanup work: CCPP metadata units and more NOAA-EMC/fv3atm#140

Merged

Cleanup work: CCPP metadata units and more NCAR/ccpp-physics#470

Merged

gold2718 approved these changes Jul 10, 2020

View reviewed changes

fix a bug to avoid reading keys in metadata2html.py

db66a81

climbfuji added 2 commits July 13, 2020 11:31

Merge branch 'metadata2html_fix' of https://github.com/mzhangw/ccpp-f…

bdd0848

…ramework into add_degrees_to_radians_conversion

scripts/conversion_tools/unit_conversion.py: make unit 'degree' singular

d71e0d5

climbfuji mentioned this pull request Jul 13, 2020

fix a bug to avoid reading keys in metadata2html.py #312

Merged

climbfuji requested a review from gold2718 July 15, 2020 15:46

gold2718 approved these changes Jul 15, 2020

View reviewed changes

climbfuji merged commit 209f1c9 into NCAR:master Jul 15, 2020

climbfuji mentioned this pull request Jul 15, 2020

Follow-up changes for PR https://github.com/NCAR/ccpp-framework/pull/307 #309

Closed

climbfuji deleted the add_degrees_to_radians_conversion branch June 27, 2022 02:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit conversion from degrees to radians, address isssue #309 #311

Add unit conversion from degrees to radians, address isssue #309 #311

climbfuji commented Jul 9, 2020 •

edited

Loading

climbfuji commented Jul 9, 2020

gold2718 commented Jul 9, 2020

climbfuji commented Jul 9, 2020

climbfuji commented Jul 9, 2020

gold2718 commented Jul 9, 2020

climbfuji commented Jul 9, 2020

climbfuji commented Jul 10, 2020

cacraigucar commented Jul 10, 2020

climbfuji commented Jul 10, 2020

gold2718 left a comment

gold2718 Jul 10, 2020

climbfuji Jul 10, 2020 •

edited

Loading

gold2718 Jul 10, 2020

grantfirl commented Jul 10, 2020

climbfuji commented Jul 10, 2020

grantfirl commented Jul 10, 2020

gold2718 commented Jul 10, 2020

climbfuji commented Jul 13, 2020

grantfirl commented Jul 13, 2020

gold2718 commented Jul 13, 2020

grantfirl commented Jul 13, 2020

gold2718 commented Jul 13, 2020

climbfuji commented Jul 13, 2020 via email

gold2718 left a comment

Add unit conversion from degrees to radians, address isssue #309 #311

Add unit conversion from degrees to radians, address isssue #309 #311

Conversation

climbfuji commented Jul 9, 2020 • edited Loading

climbfuji commented Jul 9, 2020

gold2718 commented Jul 9, 2020

climbfuji commented Jul 9, 2020

climbfuji commented Jul 9, 2020

gold2718 commented Jul 9, 2020

climbfuji commented Jul 9, 2020

climbfuji commented Jul 10, 2020

cacraigucar commented Jul 10, 2020

climbfuji commented Jul 10, 2020

gold2718 left a comment

Choose a reason for hiding this comment

gold2718 Jul 10, 2020

Choose a reason for hiding this comment

climbfuji Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

gold2718 Jul 10, 2020

Choose a reason for hiding this comment

grantfirl commented Jul 10, 2020

climbfuji commented Jul 10, 2020

grantfirl commented Jul 10, 2020

gold2718 commented Jul 10, 2020

climbfuji commented Jul 13, 2020

grantfirl commented Jul 13, 2020

gold2718 commented Jul 13, 2020

grantfirl commented Jul 13, 2020

gold2718 commented Jul 13, 2020

climbfuji commented Jul 13, 2020 via email

gold2718 left a comment

Choose a reason for hiding this comment

climbfuji commented Jul 9, 2020 •

edited

Loading

climbfuji Jul 10, 2020 •

edited

Loading