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

Add CF-compliant Time coordinate #19

Closed
wants to merge 7 commits into from

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented May 18, 2022

This variable has CF-compliant units and calendar. To accommodate the CF-compliant units, a new config option has been added to supply the reference time, 0001-01-01 by default.

@xylar xylar changed the title Add Time coordinate Add CF-compliant Time coordinate May 18, 2022
@xylar
Copy link
Collaborator Author

xylar commented May 18, 2022

Here are the results so far (using the QU240/PHC/performance test in compass):

$ ncdump -h output.nc
...
	double Time(Time) ;
		Time:long_name = "time" ;
		Time:units = "days since 0001-01-01 00:00:00" ;
		Time:calendar = "gregorian_noleap" ;
...

$ cfchecks output.nc 
CHECKING NetCDF FILE: output.nc
=====================
Using CF Checker Version 4.1.0
Checking against CF Version CF-1.8
Using Standard Name Table Version 79 (2022-03-19T15:25:54Z)
Using Area Type Table Version 10 (23 June 2020)
Using Standardized Region Name Table Version 4 (18 December 2018)

ERROR: (2.6.1): This netCDF file does not appear to contain CF Convention data.

------------------
Checking variable: Time
------------------
...

Still to do:

@xylar
Copy link
Collaborator Author

xylar commented May 18, 2022

This needs work (in QU240/PHC/daily_output_test):

$ ncdump -h forward/analysis_members/mpaso.hist.am.timeSeriesStatsDaily.0001-01-01.nc
...
	double timeDaily_avg_Time(Time) ;
		timeDaily_avg_Time:long_name = "time" ;
...

@xylar xylar force-pushed the ocn/add-cf-time branch from 83d0c89 to 04d0f5b Compare May 25, 2022 05:57
@xylar
Copy link
Collaborator Author

xylar commented May 25, 2022

Getting there:

	double timeDaily_avg_Time(Time) ;
		timeDaily_avg_Time:long_name = "time" ;
		timeDaily_avg_Time:units = "days since 0001-01-01 00:00:00" ;
		timeDaily_avg_Time:calendar = "gregorian_noleap" ;
	double timeDaily_avg_Time_bnds(Time, bnds) ;
		timeDaily_avg_Time_bnds:long_name = "time bounds" ;

But the names in output aren't Time and Time_bnds, as expected.

@xylar xylar marked this pull request as draft May 29, 2022 14:53
@xylar xylar force-pushed the ocn/add-cf-time branch from 04d0f5b to 1dbc11a Compare August 9, 2022 18:59
@xylar xylar changed the base branch from master to alternate August 29, 2022 09:21
@xylar xylar changed the base branch from alternate to master August 29, 2022 09:22
@xylar xylar marked this pull request as ready for review August 29, 2022 09:22
Copy link
Collaborator

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

I ran the QU240/PHC daily_output test case on chrysalis with intel (mpi) compiler and the Time and Time_bounds variables look as expected in the daily stats analysis member and in the output stream, when they are added as variables to that stream.

Thanks, @xylar!

@xylar
Copy link
Collaborator Author

xylar commented Aug 29, 2022

Thanks @cbegeman. I ran into trouble with the daily_output_test test case on my laptop. xarray isn't happy with the existing gregorian_noleap calendar. I'm hoping to solve that in E3SM-Project#5162 and then rebase this.

@xylar
Copy link
Collaborator Author

xylar commented Aug 29, 2022

The detailed error was:

compass calling: compass.ocean.tests.global_ocean.daily_output_test.DailyOutputTest.validate()
  in /home/xylar/code/compass/compass/compass/ocean/tests/global_ocean/daily_output_test/__init__.py

Exception raised in validate()
Traceback (most recent call last):
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 261, in decode_cf_datetime
    dates = _decode_datetime_with_pandas(flat_num_dates, units, calendar)
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 201, in _decode_datetime_with_pandas
    raise OutOfBoundsDatetime(
pandas._libs.tslibs.np_datetime.OutOfBoundsDatetime: Cannot decode times from a non-standard calendar, 'gregorian_noleap', using pandas.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 174, in _decode_cf_datetime_dtype
    result = decode_cf_datetime(example_value, units, calendar, use_cftime)
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 263, in decode_cf_datetime
    dates = _decode_datetime_with_cftime(
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 195, in _decode_datetime_with_cftime
    cftime.num2date(num_dates, units, calendar, only_use_cftime_datetimes=True)
  File "src/cftime/_cftime.pyx", line 549, in cftime._cftime.num2date
  File "src/cftime/_cftime.pyx", line 117, in cftime._cftime._dateparse
  File "src/cftime/_cftime.pyx", line 1153, in cftime._cftime.datetime.__init__
ValueError: calendar must be one of ['standard', 'gregorian', 'proleptic_gregorian', 'noleap', 'julian', 'all_leap', '365_day', '366_day', '360_day'], got 'gregorian_noleap'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/xylar/code/compass/compass/compass/run/serial.py", line 144, in run_tests
    test_case.validate()
  File "/home/xylar/code/compass/compass/compass/ocean/tests/global_ocean/daily_output_test/__init__.py", line 61, in validate
    compare_variables(
  File "/home/xylar/code/compass/compass/compass/validate.py", line 140, in compare_variables
    result = _compare_variables(
  File "/home/xylar/code/compass/compass/compass/validate.py", line 214, in _compare_variables
    ds1 = xarray.open_dataset(filename1)
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/backends/api.py", line 495, in open_dataset
    backend_ds = backend.open_dataset(
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/backends/netCDF4_.py", line 567, in open_dataset
    ds = store_entrypoint.open_dataset(
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/backends/store.py", line 27, in open_dataset
    vars, attrs, coord_names = conventions.decode_cf_variables(
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/conventions.py", line 516, in decode_cf_variables
    new_vars[k] = decode_cf_variable(
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/conventions.py", line 364, in decode_cf_variable
    var = times.CFDatetimeCoder(use_cftime=use_cftime).decode(var, name=name)
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 673, in decode
    dtype = _decode_cf_datetime_dtype(data, units, calendar, self.use_cftime)
  File "/home/xylar/miniconda3/envs/dev_compass_1.2.0-alpha.1_mpich/lib/python3.9/site-packages/xarray/coding/times.py", line 184, in _decode_cf_datetime_dtype
    raise ValueError(msg)
ValueError: unable to decode time units 'days since 0001-01-01 00:00:00' with "calendar 'gregorian_noleap'". Try opening your dataset with decode_times=False or installing cftime if it is not installed.

jonbob referenced this pull request in E3SM-Project/E3SM Sep 6, 2022
Rename gregorian_noleap to just noleap calendar in mpas-framework

The noleap is the CF-compliant name for what MPAS currently calls the
gregorian_noleap calendar. This merge changes gregorian_noleap to noleap
in the MPAS framework and in all 3 MPAS components.

The context of this change is that the config_calendar_type namelist
option is being used for the calendar attribute of the Time coordinate
in E3SM-Ocean-Discussion#19. However, the result is not making
post-processing tools (notably xarray) happy because the
gregorian_noleap calendar is not CF-compliant.

[NML]
[BFB]
jonbob referenced this pull request in E3SM-Project/E3SM Sep 7, 2022
Rename gregorian_noleap to just noleap calendar in mpas-framework

The noleap is the CF-compliant name for what MPAS currently calls the
gregorian_noleap calendar. This merge changes gregorian_noleap to noleap
in the MPAS framework and in all 3 MPAS components.

The context of this change is that the config_calendar_type namelist
option is being used for the calendar attribute of the Time coordinate
in E3SM-Ocean-Discussion#19. However, the result is not making
post-processing tools (notably xarray) happy because the
gregorian_noleap calendar is not CF-compliant.

[NML]
[BFB]
This variable has CF-compliant units and calendar.  To accommodate
the CF-compliant units, a new config option has been added to
supply the reference time, 0001-01-01 by default.
@xylar xylar changed the base branch from master to alternate September 25, 2022 04:27
@xylar xylar changed the base branch from alternate to master September 25, 2022 04:28
@xylar
Copy link
Collaborator Author

xylar commented Sep 25, 2022

Closed in favor of E3SM-Project#5202

@xylar xylar closed this Sep 25, 2022
@xylar xylar deleted the ocn/add-cf-time branch October 7, 2022 15:58
xylar pushed a commit that referenced this pull request Nov 8, 2023
activate snicar in icepack, check values
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.

2 participants