CNES / pangeo-pytide

Tidal constituents analysis in Python.
BSD 3-Clause "New" or "Revised" License
31 stars 10 forks source link

problem with time zones can create time shifts #6

Closed jerabaul29 closed 4 years ago

jerabaul29 commented 4 years ago

It seems that problems with taking care of time zones may create some time shifts. For example:

# this is just a copy of the pytide example available in section "Quick tutorial": 
# https://github.com/CNES/pangeo-pytide

import netCDF4
import pytide

import datetime

import matplotlib.pyplot as plt

import os
import time as time_module

########################################################################################################################
########################################################################################################################

# path to the git repo for pangeo-tide: https://github.com/CNES/pangeo-pytide
path_to_pangeopytide = "/home/jrmet/Desktop/Git/pangeo-pytide/"
tide_example = "tests/dataset/fes_tide_time_series.nc"

with netCDF4.Dataset(path_to_pangeopytide + tide_example) as dataset:
    time = dataset['time'][:] * 1e-6    # microseconds to epoch (seconds)
    # h = dataset['ocean'][:] * 1e-2      # cm to m # NOTE: this seems to be veery small, guess drop the 1e-2?
    h = dataset['ocean'][:]             # TODO: report

# seems that datetimes are needed TODO: report
time_as_datetimes = [datetime.datetime.fromtimestamp(crrt_timestamp) for crrt_timestamp in time]

# build pytide wave table with all modes
wt = pytide.WaveTable()

# compute the nodal modulations corresponding to the times
f, vu = wt.compute_nodal_modulations(time_as_datetimes)

# build modes table from the available records
w = wt.harmonic_analysis(h, f, vu)

# predict over a time range TODO: why print datetimes?
hp = wt.tide_from_tide_series(time, w)

# show the results

plt.figure()

plt.plot(time_as_datetimes, h, label="data")
plt.plot(time_as_datetimes, hp + 0.1, label="model fit shifted 0.1")
plt.title("results using sytem TZ: {}".format(time_module.tzname))

plt.legend(loc="lower right")

plt.savefig("system_TZ")

plt.show()

########################################################################################################################
########################################################################################################################

# if not making sure that the interpreter uses the UTC TZ, some time shifts are introduced
# TODO: report

# Make sure the interpreter is in UTC in all the following
os.environ["TZ"] = "UTC"
time_module.tzset()

# path to the git repo for pangeo-tide: https://github.com/CNES/pangeo-pytide
path_to_pangeopytide = "/home/jrmet/Desktop/Git/pangeo-pytide/"
tide_example = "tests/dataset/fes_tide_time_series.nc"

with netCDF4.Dataset(path_to_pangeopytide + tide_example) as dataset:
    time = dataset['time'][:] * 1e-6    # microseconds to epoch (seconds)
    # h = dataset['ocean'][:] * 1e-2      # cm to m # NOTE: this seems to be veery small, guess drop the 1e-2?
    h = dataset['ocean'][:]             # TODO: report

# seems that datetimes are needed TODO: report
time_as_datetimes = [datetime.datetime.fromtimestamp(crrt_timestamp) for crrt_timestamp in time]

# build pytide wave table with all modes
wt = pytide.WaveTable()

# compute the nodal modulations corresponding to the times
f, vu = wt.compute_nodal_modulations(time_as_datetimes)

# build modes table from the available records
w = wt.harmonic_analysis(h, f, vu)

# predict over a time range TODO: why print datetimes?
hp = wt.tide_from_tide_series(time, w)

# show the results

plt.figure()

plt.plot(time_as_datetimes, h, label="data")
plt.plot(time_as_datetimes, hp + 0.1, label="model fit shifted 0.1")
plt.title("results force timezone utc with os.environ['TZ'] = 'UTC")

plt.legend(loc="lower right")

plt.savefig("UTC_TZ")

plt.show()

Produces the figures:

system_TZ

UTC_TZ

How do you think this should be fixed? I think this is probably related to some points discussed in #5 , i.e. choosing between datetimes and timestamps, and making sure internal conversions are handled correctly. One thing I noticed: the compute_nodal_modulations function seems to accept only timezone unaware datetimes. Wonder if it may be simpler to use timezone aware datetimes, and maybe accept only UTC TZ? This way, the user cannot shoot himself / herself in the foot.

Once we have discussed this and agree, I will be happy to work on a pull request, if you want.

fbriol commented 4 years ago

The latest version uses an internal function to decode dates. This should normally solve the problem. Because the Python function that decodes performs date conversion from UNIX dates (number of seconds elapsed since 1970) is time zone sensitive...