Opened 13 years ago

Closed 10 years ago

#283 closed defect (fixed)

Check consistency between start_time and year,month,day etc in ropp2bufr

Reported by: Ian Culverwell Owned by: frdo,idculv
Priority: normal Milestone: 8.0
Component: ropp_io Version: 6.0
Keywords: Cc:

Description

Axel von Engeln (EUM), while working on netcdf-4-to-bufr converters (see ticket #280), found inconsistent results when converting to bufr format because start_date didn't tally with the other time info in the file.

He suggests we get ropp2bufr to check these are consistent in the ropp file before converting, and issue a (warning?, failure?) if not.

Attachments (2)

RE_ROPP_Start_Time_Accuracy.mail (5.5 KB ) - added by Ian Culverwell 10 years ago.
yaros2ropp.nc (31.2 KB ) - added by Ian Culverwell 10 years ago.

Download all attachments as: .zip

Change history (16)

comment:1 by Ian Culverwell, 13 years ago

We might also want to check the time info in the filename (if available) - but this should probably only raise a warning, not a failure, I think.

comment:2 by Dave Offiler, 13 years ago

Owner: changed from Ian Culverwell to Dave Offiler
Status: newassigned

We can check and warn (in the lower-level generic reader - this isn't obviously a specific BUFR conversion issue), but the real problem lies with whatever created the netCDF file. If this was ROPP code (e.g. using ropp_io_write()), then we need to fix this upstream of the BUFR conversion. If (as is more likely) the problem file is netCDF-4, then Axel should have a word with whowever created it (Christian?).

comment:3 by Dave Offiler, 11 years ago

Milestone: 7.07.1
Status: assignedaccepted

Lack of time to investigate for 7.0 - have to postpone to 7.1

comment:4 by Ian Culverwell, 11 years ago

Milestone: 7.18.0

comment:5 by Ian Culverwell, 11 years ago

Type: defecttask

We agreed to make this a task

by Ian Culverwell, 10 years ago

comment:6 by Ian Culverwell, 10 years ago

Added email exchange on the subject between AvE and IC.

by Ian Culverwell, 10 years ago

Attachment: yaros2ropp.nc added

comment:7 by Ian Culverwell, 10 years ago

Also added source file. Other files can be found in my $ROPP_ROOT/Tickets/Tests_6p0/start_time.

comment:8 by Dave Offiler, 10 years ago

Axel's original email (now attached to this ticket) clarifies that (a) the correct variable is start_time not start_date which does not exist anywhere in the ROPP code or netCDF (this was just a typo when raising the ticket) and (b) the discrepancy appears in start_time losing the fractional (millisecond) which is non-zero in the input file but truncated to zero in the output file.

As a check:

Using ropp_io/data/ropp_test.nc: ncdump shows:

start_time = 303861487.813993 second = 7 msec = 814

run ropp2bufr > ropp_test.bufr decbufr shows:

SECOND = 7.814

run bufr2ropp > ropp_test.nc2 ncdump shows:

start_time = 303861487.813993 second = 7 msec = 814

So for this standard ROPP netCDF test file, start_time and the time elements (and in particular Msecs) are consistent and remain so after an encode/decode cycle.

It is a simple matter to modify the above test netCDF file to set msec=0 (but leaving start_time alone) and on re-running the above cycle, show that the output netCDF (correctly) retains msec=0 but with start_time truncated (to effectively a whole second).

This would be expected, since inspecting the code:

  • when inputting a ROPP netCDF file, start_time is read into a temporary variable time but is not saved to the ROPP data structure; only the 7 elements are saved to the DTocc sub-structure;
  • on encoding, the separate timestamp elements are mapped to BUFR parameters with seconds and msecs combined into a single float;
  • on decoding, the BUFR variables are mapped back to the 7 DTocc elements;
  • on write, start_time is first calculated from the 7 elements then all 8 variables are output to the netCDF file.

Hence at no place in the ROPP_IO code (in tools ropp2bufr, eum2bufr or bufr2ropp or the lower level modules) is start_time different from the 7-element timestamps. As long as ROPP I/O routines are used to write netCDF files, start_time will always be consistent with the 7-element timestamp.

The problem arises when the source file has not been written via ROPP routines and the start_time and the separate elements are not consistent to start with. Clearly then, if an input file has msec=0 but an inconsistent, msec/=0 fraction in start_time, and that file is cycled, the output file will (correctly) have msec=0 but now with a consistent start_time also having a zero-msec fractional part. This would apply to any element having any different effective value compared with the start_time value.

Since ROPP code makes no use whatever of start_time, it isn't checked against the separate elements. In this sense it is not fool-proof against user-error. It would be a simple matter to convert the elements to a time since and test this against start_time and issue a WARNING if thy differ by 1ms or more.

Agreed with Ian that although (ideally) the 7-element timestamp ought to be checked and if invalid to fall back on start_time (if itself is valid), we will just add a consistency check for now.

comment:9 by Dave Offiler, 10 years ago

Current ropp2bufr:

$ $ROPP_ROOT/ifort/bin/ropp2bufr ropp_test1.nc -o ropp_test1.bufr

---------------------------------------------------------------------
                     ROPP to BUFR Encoder
---------------------------------------------------------------------

INFO (from ropp2bufr):  Reading  ROPP data from ropp_test1.nc
INFO (from ropp2bufr):  Encoding profile    1 : OC_20090817215807_META_G027_DMI_
INFO (from ropp2bufr):  Total of  14742 bytes written to ropp_test1.bufr
INFO (from ropp2bufr):  Generated 1 BUFR message to ropp_test1.bufr

With modified reader:

$ ../tools/ropp2bufr ropp_test1.nc -o ropp_test1.bufr

---------------------------------------------------------------------
                     ROPP to BUFR Encoder
---------------------------------------------------------------------

INFO (from ropp2bufr):  Reading  ROPP data from ropp_test1.nc

WARNING (from ropp_io_read_ncdf_get):  'start_time' and 7-element times differ by 0.814 seconds - using 7-element time
INFO (from ropp2bufr):  Encoding profile    1 : OC_20090817215807_META_G027_DMI_
INFO (from ropp2bufr):  Total of  14742 bytes written to ropp_test1.bufr
INFO (from ropp2bufr):  Generated 1 BUFR message to ropp_test1.bufr

Of course the new version still has the msec truncation, it just issues a warning (to be interpreted as 'this is a user-error').

comment:10 by Dave Offiler, 10 years ago

Added date/time element checking to the reader (similar to the writer) with fall-back to start_time. This is to cover a use-case where a user can't or won't set the 7-element (calendar) timestamp, but only sets the (timesince) start_time. Previously we have assumed an invalid date/time to be a Never event.

Modified input file to set Year = -999.

Current ropp2bufr:

$ $ROPP_ROOT/ifort/bin//ropp2bufr ropp_test2.nc -o ropp_test2.bufr

---------------------------------------------------------------------
                     ROPP to BUFR Encoder
---------------------------------------------------------------------

INFO (from ropp2bufr):  Reading  ROPP data from ropp_test2.nc
INFO (from ropp2bufr):  Encoding profile    1 : OC_99990817215807_META_G027_DMI_
  7-TH VALUE (004001)       9999. IN 12 BITS - SET TO MISSING         310026
INFO (from ropp2bufr):  Total of  14742 bytes written to ropp_test2.bufr
INFO (from ropp2bufr):  Generated 1 BUFR message to ropp_test2.bufr

Note that while the invalid Year element has been trapped, it only gets converted to another 'missing data' value (-999 -> 9999) which the BUFR kernel encoder then warns about but converts to it's own internal missing data value (all bits set). On decode this would still be 'missing' and of no use to the end-user.

With modified reader:

$ ../tools/ropp2bufr ropp_test2.nc -o ropp_test2.bufr

---------------------------------------------------------------------
                     ROPP to BUFR Encoder
---------------------------------------------------------------------

INFO (from ropp2bufr):  Reading  ROPP data from ropp_test2.nc

WARNING (from ropp_io_read_ncdf_get):  One or more 7-element times are invalid - using 'start_time' instead
INFO (from ropp2bufr):  Encoding profile    1 : OC_20090817215807_META_G027_DMI_
INFO (from ropp2bufr):  Total of  14742 bytes written to ropp_test2.bufr
INFO (from ropp2bufr):  Generated 1 BUFR message to ropp_test2.bufr

Here, start_time (valid) has been substituted, so BUFR doesn't get the bad Year element. As a bonus, the non-zero Msec fractional value gets encoded in place of the zeroed msec element.

Now modify start_time = -999.

Current ropp2bufr - as above.

Modified reader:

$ ../tools/ropp2bufr ropp_test2.nc -o ropp_test2.bufr

---------------------------------------------------------------------
                     ROPP to BUFR Encoder
---------------------------------------------------------------------

INFO (from ropp2bufr):  Reading  ROPP data from ropp_test2.nc

FATAL ERROR (from ropp_io_read_ncdf_get):  Occultation start time is invalid

Here, neither start_time or the 7-element time are valid, so we abort the conversion (an observation with no valid time is useless).

It could be argued that a fatal error (abort) is too harsh, and (for multi-files anyway), a warning should be isued and that profile merely skipped as later ones may be OK. But I think on balance that even one no-time should be killed.

comment:11 by Dave Offiler, 10 years ago

Consequence of over-checking and aborting at this low level is that the t_ropp2ropp -r test fails due to the (intended) invalid start_time and DTocc elements, thus stopping the usual Q/C range checking and the (new) non-zero exit code trips make to abort further tests.

After a re-think, the fatal error is inappropriate here, so the on-read check should merely:

  • check if both timestamps are valid and they differ by >= 1ms - in which case just use DTocc as before and issue a warning;
  • if DTocc is invalid but start_time is valid (use-case where user sets the latter but not the former when not using the ROPP netCDF writer), then convert start_time to DTocc and issue a warning;
  • if neither is valid, do nothing at all, on the basis that coordinates (temporal and spatial) should be checked by generic Q/C at a higher level and an occultation flagged as unusable, rejected, processing aborted or whatever action is appropriate, by the application.

Code amended as above and re-tested with above hacked netCDF files; the two warning cases are correctly detected and flagged, but the 'both invalid' case now just lets the bad timestamp through. This is then caught by the low-level BUFR encoder (as previously). The latter merely converts the bad value to the usual BUFR 'missing data value'; the encode is otherwise successful and the BUFR file is generated.

Whether the encoder (or other tools) should just pass invalid coords though or reject them is perhaps for a separate ticket.

Mods checked into branch do_exitcodes as [4250].

comment:12 by Ian Culverwell, 10 years ago

Component: ROPP (all)ropp_io

Tested on yaros2ropp.nc, which has

day[0]=31 

hour[0]=0 

minute[0]=7 

month[0]=1 

msec[0]=0 

second[0]=7 

start_time[0]=381283627.06 

year[0]=2012 

and got

WARNING (from ropp_io_read_ncdf_get):  'start_time' and yr/mo/dy/hr/mn/sc/ms timestamps differ by 0.060 seconds - using yr/../ms timestamp

as expected.

Checked the other combinations of start_time and yr/mo/.../ms. OK.

Note the following effect. If we set msec=-1 in the original ROPP file (ncap2 -s"msec=msec*0-1" yaros2ropp.nc -O yaros2ropp4.nc), then at ROPP7.1 we get

YEAR                                                   YEAR                2012
 MONTH                                                 MONTH                  1
 DAY                                                   DAY                   31
 HOUR                                                  HOUR                   0
 MINUTE                                                MINUTE                 7
 SECOND                                                SECOND            16.999

in the BUFR file. Don't know where the 16.999 comes from, but it's wrong.

At ROPP8.0. by contrast, we get

WARNING (from ropp_io_read_ncdf_get):  One or more of yr/mo/dy/hr/mn/sc/ms times are invalid - using 'start_time' instead

as expected, and

 YEAR                                                  YEAR                2012
 MONTH                                                 MONTH                  1
 DAY                                                   DAY                   31
 HOUR                                                  HOUR                   0
 MINUTE                                                MINUTE                 7
 SECOND                                                SECOND             7.060

in the BUFR file, as desired. So this mod will fix 'silent' errors - the worst kind.

comment:13 by Ian Culverwell, 10 years ago

Owner: changed from Dave Offiler to frdo,idculv
Status: acceptedassigned
Type: taskdefect

Add in the analogous logic for subroutine ropp_io_read_ncdf_get_rodata_2d.

Tests out OK.

No need to do similarly for subroutine ropp_io_read_eumdata as it reads the DTocc info from the variables utc_start_absdate and utc_start_abstime ONLY.

Nor do we have to worry about the UCAR file date-time reading either, because, to quote the code,

! COSMIC has no processing date, set to current utc date/time
  CALL Date_and_Time_UTC ( Values=DTnow )
  data%DTpro%Year   = DTnow(1)
  data%DTpro%Month  = DTnow(2)
  data%DTpro%Day    = DTnow(3)
  data%DTpro%Hour   = DTnow(5)
  data%DTpro%Minute = DTnow(6)
  data%DTpro%Second = DTnow(7)
  data%DTpro%Msec   = DTnow(8)

Happy with this, so committing to ROPP8.0 at [4252].

comment:14 by Dave Offiler, 10 years ago

Resolution: fixed
Status: assignedclosed

Reading UCAR netCDF isn't a problem because there aren't the two (supposedly identical, just different representations for user convenience) data timestamps in ROPP netCDF. The above code snippet is to cover for a non-present processing_time in UCAR netCDF.

OK, looks like we've cracked the problem Axel raised, so closing it. There potentially remains a downstream problem if any 'coordinate' parameters are missing (should be a 'never' event) - to be dealt with separately if it ever occurs.

Note: See TracTickets for help on using tickets.