Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Time truncated on conversion to CalendarDate #73

Closed
jonescc opened this issue Aug 25, 2014 · 4 comments
Closed

Time truncated on conversion to CalendarDate #73

jonescc opened this issue Aug 25, 2014 · 4 comments

Comments

@jonescc
Copy link

jonescc commented Aug 25, 2014

We are currently having an issue with the embedded ncWMS server in our thredds instance where date/time values reported in the GetCapabilities statement and in Godiva do not reflect the closest date/time to the value recorded in the netcdf dataset. I believe this is an issue with the netcdf java library as explained below, rather than with ncWMS itself. This is also an issue in standalone ncWMS and is causing us issues downstream where we are using the date/time reported by ncWMS for further work (its not just a cosmetic issue).

For example, for the netcdf file

http://thredds-6-nsp-mel.aodn.org.au/thredds/fileServer/IMOS/ACORN/gridded_1h-avg-current-map_QC/ROT/2014/03/13/IMOS_ACORN_V_20140313T023000Z_ROT_FV01_1-hour-avg.nc 

the time value is 23447.104166666664 (days since 1950-01-01 00:00:00 UTC). If I use matlab to calculate the date/time value by adding this duration to the base date, matlab gives me 14-03-13 02:30 which is what we are expecting:

>> datestr(23447.104166666664 + datenum('01-01-1950 00:00:00'), 'dd-mmm-yyyy HH:MM:SS.FFF')

ans =

13-Mar-2014 02:30:00.000

nctools also reports the date/time as 2014-03-13 02:30 :

ggalibert@5-nsp-mel:/mnt/opendap/1/IMOS/opendap/ACORN/gridded_1h-avg-current-map_QC/ROT/2014/03/13$ ncdump -t -v TIME IMOS_ACORN_V_20140313T023000Z_ROT_FV01_1-hour-avg.nc | grep 'TIME = "' TIME = "2014-03-13 02:30" ;

But the embedded ncWMS reports the date/time as 2014-03-13 02:29:99.999:

image

Debugging this in ncWMS I found that ncWMS uses the ucar.nc2.time.CalendarDate add method to calculate this date/time.

The reason why ucar.nc2.time.CalendarDate add returns 2014-03-13 02:29:99.999 is because it casts the time in milliseconds calculated as a double to a long, in the process truncating it:

      case Day:
        return new CalendarDate(cal, dateTime.plus( (long) (value * 86400  * 1000) ));

it would make more sense to round it to the nearest millisecond as this gives us the closest date/time to the stored double value as follows:

      case Day:
        return new CalendarDate(cal, dateTime.plus( Math.round(value * 86400  * 1000) ));

(same applies to other calculations in this method).

Any chance of getting this fixed? I can prepare a pull request if you would like.

@JohnLCaron
Copy link
Collaborator

What version of netcdf-java are you using?

On Sun, Aug 24, 2014 at 11:53 PM, Craig Jones notifications@github.com
wrote:

We are currently having an issue with the embedded ncWMS server in our
thredds instance where date/time values reported in the GetCapabilities
statement and in Godiva do not reflect the closest date/time to the value
recorded in the netcdf dataset. I believe this is an issue with the netcdf
java library as explained below, rather than with ncWMS itself. This is
also an issue in standalone ncWMS and is causing us issues downstream where
we are using the date/time reported by ncWMS for further work (its not just
a cosmetic issue).

For example, for the netcdf file

http://thredds-6-nsp-mel.aodn.org.au/thredds/fileServer/IMOS/ACORN/gridded_1h-avg-current-map_QC/ROT/2014/03/13/IMOS_ACORN_V_20140313T023000Z_ROT_FV01_1-hour-avg.nc

the time value is 23447.104166666664 (days since 1950-01-01 00:00:00 UTC).
If I use matlab to calculate the date/time value by adding this duration to
the base date, matlab gives me 14-03-13 02:30 which is what we are
expecting:

datestr(23447.104166666664 + datenum('01-01-1950 00:00:00'), 'dd-mmm-yyyy HH:MM:SS.FFF')

ans =

13-Mar-2014 02:30:00.000

nctools also reports the date/time as 2014-03-13 02:30 :

ggalibert@5-nsp-mel:/mnt/opendap/1/IMOS/opendap/ACORN/gridded_1h-avg-current-map_QC/ROT/2014/03/13$
ncdump -t -v TIME IMOS_ACORN_V_20140313T023000Z_ROT_FV01_1-hour-avg.nc |
grep 'TIME = "'
TIME = "2014-03-13 02:30" ;

But the embedded ncWMS reports the date/time as 2014-03-13 02:29:99.999:

[image: image]
https://cloud.githubusercontent.com/assets/1860215/4026338/546a9600-2c16-11e4-927c-b637ceca154b.png

Debugging this in ncWMS I found that ncWMS uses the
ucar.nc2.time.CalendarDate add method to calculate this date/time.

The reason why ucar.nc2.time.CalendarDate add returns 2014-03-13
02:29:99.999 is because it casts the time in milliseconds calculated as a
double to a long, in the process truncating it:

  case Day:
    return new CalendarDate(cal, dateTime.plus( (long) (value * 86400  * 1000) ));

it would make more sense to round it to the nearest millisecond as this
gives us the closest date/time to the stored double value as follows:

  case Day:
    return new CalendarDate(cal, dateTime.plus( Math.round(value * 86400  * 1000) ));

(same applies to other calculations in this method).

Any chance of getting this fixed? I can prepare a pull request if you
would like.


Reply to this email directly or view it on GitHub
#73.

@jonescc
Copy link
Author

jonescc commented Aug 25, 2014

Problem occurs in the latest version of Reading ncWMS which uses 4.3.22 and in earlier versions. Code looks the same in master.

@jonescc
Copy link
Author

jonescc commented Aug 25, 2014

Make that, the code looks the same in other branches - the repository defaulted to target-4.3.22, but the code looks the same in other branches including 4.5.3.

Changes which appear to fix this problem for us (in 4.3.17) are:

https://github.com/aodn/thredds/pull/1/files

@JohnLCaron
Copy link
Collaborator

Hi Craig:

These changes look great. I am adding to 4.3.23 (prob released today) and
4.5.3 (prob released in a day or two).

I appreciate you making these contributions.

John

On Mon, Aug 25, 2014 at 4:27 PM, Craig Jones notifications@github.com
wrote:

Make that, the code looks the same in other branches - the repository
defaulted to target-4.3.22, but the code looks the same in other branches
including 4.5.3.

Changes which appear to fix this problem for us (in 4.3.17) are:

https://github.com/aodn/thredds/pull/1/files


Reply to this email directly or view it on GitHub
#73 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants