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

Allow variable values in ocean_month.py #208

Closed
forsyth2 opened this issue Mar 9, 2022 · 3 comments · Fixed by #219
Closed

Allow variable values in ocean_month.py #208

forsyth2 opened this issue Mar 9, 2022 · 3 comments · Fixed by #219
Assignees
Labels
semver: bug Bug fix (will increment patch version)

Comments

@forsyth2
Copy link
Collaborator

forsyth2 commented Mar 9, 2022

In zppy/templates/ocean_month.py, Lines 32-35, 10 is hard-coded:

for y in range(start_yr, end_yr, 10):

    year1 = y
    year2 = y + 10 - 1

This was causing issues addressing #128 because the tests were using fewer than 10 years.

A related issue, on line 29: tunits = "days since 0001-01-01 00:00:00", which I believe is only true if we're running from year 0001 and not say 1850.

@forsyth2 forsyth2 added the semver: bug Bug fix (will increment patch version) label Mar 9, 2022
@forsyth2 forsyth2 self-assigned this Mar 9, 2022
@forsyth2 forsyth2 changed the title Allow variable years in ocean_month.py Allow variable values in ocean_month.py Mar 9, 2022
@xylar
Copy link
Contributor

xylar commented Mar 9, 2022

Another issue is that these constants should come from mpas_tools.cime.constants instead of being hard-coded:

# Ocean constants
# specific heat [J/(kg*degC)]
cp = 3.996e3
# [kg/m3]
rho = 1026.0
fac = rho * cp

from mpas_tools.cime.constants import constants
...
# specific heat [J/(kg*degC)]
cp = constants['SHR_CONST_CPSW']
# [kg/m3]
rho = constants['SHR_CONST_RHOSW']
fac = rho * cp

This will require adding a new dependency on mpas_tools.

@xylar
Copy link
Contributor

xylar commented Mar 9, 2022

A related issue, on line 29: tunits = "days since 0001-01-01 00:00:00", which I believe is only true if we're running from year 0001 and not say 1850.

I don't think this is a problem. It might be more aesthetic to have:

tunits = f"days since {start_yr:d04}-01-01 00:00:00"

but your later calls to date2num( take tunits as an argument so they will work as expected regardless of the reference year.

@xylar
Copy link
Contributor

xylar commented Mar 9, 2022

year2 = y + 10 - 1

Presumably this should be the following?

year2 = min(y + 10 -1, end_yr)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: bug Bug fix (will increment patch version)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants