Opened 4 years ago

Closed 4 years ago

#682 closed task (fixed)

EUMETSAT's changes for ROPP10.0

Reported by: Ian Culverwell Owned by: Ian Culverwell
Priority: normal Milestone: 10.0
Component: ROPP(all) Version: 9.0
Keywords: Cc:

Description

We agreed to incorporate a number of last-minute changes to ROPP-10 from EUMETSAT, because

  • it's their code; and
  • they and DMI are the only likely users of the code that they propose changing.

DMI are happy about this.

The tarball of changes, eumetsat-changes-ropp-v10.0.zip, is attached. We will go through them one-by-one.

Attachments (11)

eumetsat-changes-ropp-v10.0.zip (148.4 KB ) - added by Ian Culverwell 4 years ago.
eumetsat-changes-ropp-v10.0.zip
GRAS_1B_M01_20200817073906Z_20200817074016Z_N_T_20200817102018Z_G09_NN.nc (4.2 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M01_20200817073906Z_20200817074016Z_N_T_20200817102018Z_G09_NN.nc
GRAS_1B_M01_20200817074017Z_20200817074117Z_N_T_20200817102018Z_G30_NN.nc (3.6 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M01_20200817074017Z_20200817074117Z_N_T_20200817102018Z_G30_NN.nc
GRAS_1B_M02_20200816132527Z_20200816133124Z_N_T_20200816162901Z_G07_NN.nc (9.3 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M02_20200816132527Z_20200816133124Z_N_T_20200816162901Z_G07_NN.nc
GRAS_1B_M02_20200816133009Z_20200816133500Z_N_T_20200816162901Z_G05_NN.nc (8.2 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M02_20200816133009Z_20200816133500Z_N_T_20200816162901Z_G05_NN.nc
GRAS_1B_M03_20200816143505Z_20200816143709Z_N_T_20200816181517Z_G02_NN.nc (3.6 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M03_20200816143505Z_20200816143709Z_N_T_20200816181517Z_G02_NN.nc
GRAS_1B_M03_20200816143944Z_20200816144149Z_N_T_20200816181517Z_G28_NN.nc (5.7 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M03_20200816143944Z_20200816144149Z_N_T_20200816181517Z_G28_NN.nc
GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN.nc (9.8 MB ) - added by Ian Culverwell 4 years ago.
GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN.nc
Radio_Occultation_Level_1_Product_Format_Specification.pdf (291.1 KB ) - added by Ian Culverwell 4 years ago.
Radio_Occultation_Level_1_Product_Format_Specification.pdf
PPF_445.png (43.3 KB ) - added by Ian Culverwell 4 years ago.
PPF_445.png
PPF_451.png (46.2 KB ) - added by Ian Culverwell 4 years ago.
PPF_451.png

Change history (43)

comment:1 by Ian Culverwell, 4 years ago

Setting ECCODES_BUFR_SET_TO_MISSING_IF_OUT_OF_RANGE=1 locally when using the ecCodes package

These changes include the new file ropp_utils/common/system.f90, and modifications to ropp_utils/build/Makefile.am, ropp_io/tools/ropp2bufr_eccodes.f90 and ropp_io/tools/eum2bufr_eccodes.f90. We have already included these. Chris's mods also include an analogous change to the BUFR reading program, ropp_io/tools/bufr2ropp_eccodes.f90. We choose not to incldue this, as it is not necessary, and would trip up a ROPP user who just wants to read BUFR files but is unable, for some reason, to install setenv.

comment:2 by Ian Culverwell, 4 years ago

Changes to ropp_io/ropp/ropp_io_types.f90

1) CM says

    REAL(wp), DIMENSION(2)          :: shum        = (/  -20.0_wp,   50.0_wp /)

whereas we reverted this to

    REAL(wp), DIMENSION(2)          :: shum        = (/    0.0_wp,   50.0_wp /)

after the beta version was released. (The valid range of shum is now only relaxed to (-20, 50) g/kg if check_qmin is .FALSE., which, by default, it isn't.) So leave it as it is.

2) CM says

    CHARACTER(len = 160):: thin_method       = "UNKNOWN" ! Profile thinning method

whereas we say

    CHARACTER(len = 80) :: thin_method       = "UNKNOWN" ! Profile thinning method

Why did Chris change this?

comment:3 by Ian Culverwell, 4 years ago

Changes to ropp_io/ropp/ropp_io_read_ncdf_get.f90

1) Addition of COSMIC-1 satellite IDs in SUBROUTINE ropp_io_read_eumdata:

  IF ( readstr == 'C01' ) data%leo_id = 'C001'
  IF ( readstr == 'C02' ) data%leo_id = 'C002'
  IF ( readstr == 'C03' ) data%leo_id = 'C003'
  IF ( readstr == 'C04' ) data%leo_id = 'C004'
  IF ( readstr == 'C05' ) data%leo_id = 'C005'
  IF ( readstr == 'C06' ) data%leo_id = 'C006

I didn't know EUMETSAT distributed C1 data in this format. Anyway, seems an unobjectionable change, and compiles OK, so commit change at r6413.

comment:4 by Ian Culverwell, 4 years ago

2) Some code to read format_version attribute committed at r6414.

comment:5 by Ian Culverwell, 4 years ago

3) The proposed change from

  ! setting or rising
  CALL ncdf_getatt('/data/occultation/occultation_type', readstr)

to

  ! setting or rising
  IF (data%FmtVersion < '13.0') THEN
    CALL ncdf_getatt('/data/occultation/occultation_type', readstr)
  ELSE
    CALL ncdf_getvar('/data/occultation/occultation_type', readstr)
  ENDIF

in SUBROUTINE ropp_io_read_eumdata in ropp_io_read_ncdf_get.f90 has been included at r6415, although I cannot check it without a test file in the new format. (Note that < is defined for character strings in Fortran, and in this case will work the same as for numbers, i.e. '11.0' < '13.0' is .TRUE..)

comment:6 by Ian Culverwell, 4 years ago

4) Some generalisations of the long names of the Tx / Rx pos/vel variables in EUM files were committed at r6416.

comment:7 by Ian Culverwell, 4 years ago

5) There's a clash with the check that the closed loop phase measurements are OK.

Chris's code:

        IF (data%FmtVersion < '13.0') THEN
          CALL ncdf_getvar('/quality/cl_snr_ca_ok', readbyte1)
          CALL ncdf_getvar('/quality/cl_snr_p2_ok', readbyte2)
        ELSE
          CALL ncdf_getvar('/quality/snr_l1_ok', readbyte1)
          CALL ncdf_getvar('/quality/snr_l2_ok', readbyte2)
        ENDIF

        IF ((readbyte1 == 0) .AND. (readbyte2 == 0) ) THEN

ROPP10.0:

        CALL ncdf_getvar('/quality/cl_snr_ca_ok', readbyte1)
        CALL ncdf_getvar('/quality/cl_snr_p1_ok', readbyte2)
        CALL ncdf_getvar('/quality/cl_snr_p2_ok', readbyte3)

        IF ((readbyte1 == 0) .AND. (readbyte2 == 0) .AND. (readbyte3 == 0)) THEN

The check on cl_snr_p1_ok has been in the code since at least ROPP-9.1, so why has Chris removed it? Does the new variable snr_l1_ok cover both ca and p1?

To be discussed.

comment:8 by Ian Culverwell, 4 years ago

6) external_navbits_applied updated for newer files at r6418.

Should

      IF ( TRIM(ADJUSTL(getlevel1a)) == 'cl+rs' ) THEN
        sampling_mode              = 'raw_sampling'
        IF (data%FmtVersion < '13.0') THEN
          data_available           = 'rs_data_available'
          external_navbits_applied = 'rs_external_navbits_applied'
        ELSE
          data_available           = 'ol_data_available'
          external_navbits_applied = 'external_navbits_ok'
        ENDIF
        ts_olrs = 0.001_wp ! expected (approximate) time in seconds between raw sampling samples
      ENDIF

really be

      IF ( TRIM(ADJUSTL(getlevel1a)) == 'cl+rs' ) THEN
        sampling_mode              = 'raw_sampling'
        IF (data%FmtVersion < '13.0') THEN
          data_available           = 'rs_data_available'
          external_navbits_applied = 'rs_external_navbits_applied'
        ELSE
          data_available           = 'rs_data_available'
          external_navbits_applied = 'external_navbits_ok'
        ENDIF
        ts_olrs = 0.001_wp ! expected (approximate) time in seconds between raw sampling samples
      ENDIF

? Impossible to know without a test file. Ask Chris to confirm/deny.

comment:9 by Ian Culverwell, 4 years ago

7) Closed loop lev1a variables like exphase_ca renamed to things like exphase_1c in newer file versions. Change committed at r6419. Cannot check without test file.

comment:10 by Ian Culverwell, 4 years ago

8) There's an issue with the ol/rs phases and snrs: currently we read in uncorrected ones like i_ca_uncorr, but these are unavailable in newer data files. I think the thing to do is to uncorrect the corrected i_ca and q_ca in the new files by multiplying by the navbits and proceeding as we do now. But I don't know how to combine internal and external navbits. (If the external navbits are missing, they are set equal to the internal ones, so multiplying by both wouldn't have any effect.) Stig also mentioned the possibility of just setting the navbits to zero if I & Q are already corrected, but I think we'll end up with different phase_L1s if we use the corrected navbits. This needs discussing. Meanwhile, implement the incomplete changes at r6420.

comment:11 by Ian Culverwell, 4 years ago

9) The name of the open loop L1 excess phase variable, which is read from the file if getdirect is in force, is different in the new files. Change committed at r6421. Cannot check without test file.

comment:12 by Ian Culverwell, 4 years ago

10) The raw sampling corrected i_ca and q_ca, needed to calculate the properly averaged SNR, have different names in the new files. (Untested) change committed at r6422. (This includes some cosmetic reformatting.)

NB: Chris says that EUMETSAT have now fixed the problem that this code section was designed to solve. When we know the version where this was fixed, we should wrap the averaging step in an IF ( vn < vn0 ) THEN ... ENDIF construction. Chris: please tell us vn0.

comment:13 by Ian Culverwell, 4 years ago

11) Untested code to handle the absence of fsi_done and go_done in newer files committed at r6424.

comment:14 by Ian Culverwell, 4 years ago

12) Untested code to handle the (lev1b) bangles committed at r6426. Why is the version testing written in terms of

      IF (ncdf_isvar(TRIM(ddir)//'bangle_ca')) THEN  !For backward compatibility
        CALL ncdf_getvar(TRIM(ddir)//'bangle_ca',  data%Lev1b%bangle_L1,  &
        units=data%Lev1b%units%bangle)
      ELSE
        CALL ncdf_getvar(TRIM(ddir)//'bangle_l1',  data%Lev1b%bangle_L1,  &
          units=data%Lev1b%units%bangle)
      ENDIF

(etc) rather than

      IF (data%FmtVersion < '13.0') THEN
        CALL ncdf_getvar(TRIM(ddir)//'bangle_ca',  data%Lev1b%bangle_L1,  &
        units=data%Lev1b%units%bangle)
      ELSE
        CALL ncdf_getvar(TRIM(ddir)//'bangle_l1',  data%Lev1b%bangle_L1,  &
          units=data%Lev1b%units%bangle)
      ENDIF

(etc), as it is for all the other changes?

comment:15 by Ian Culverwell, 4 years ago

13) Untested code to read global attributes/variables committed at r6427.

comment:16 by Ian Culverwell, 4 years ago

14) Untested changes to the creating of DTpro committed at r6429.

Chris: what are

    WRITE(readstr, *) readreal 

and

  data%FmtVersion       = ThisFmtVer

for?

comment:17 by Ian Culverwell, 4 years ago

15) (Cosmetic changes made at r6430.)

comment:18 by Ian Culverwell, 4 years ago

Changes to ropp_io/bufr/bufr2ropp_mod.f90

1) Need to relax the tolerance on the bending angle frequency checks, because GeoOptics assign a nominal L1 frequency of 1.6 GHz in their BUFR files instead of the far more sensible 1.5 GHz that ROPP assumes. (Note that f1 ~ 1.575 GHz, which is obviously much closer to 1.5 GHz than to 1.6 GHz. Ahem.) Likewise for L2/L5. Works OK, so change committed at r6409.

comment:19 by Ian Culverwell, 4 years ago

Changes to ropp_io/ncdf

1) These are needed to read single string attributes (netCDF type NF90_STRING), which are used in the new PPF format but which are not supported by the current netCDF fortran library (version 4.5.2). The key new routine is ropp_io/ncdf/nf90_get_att_string.f90, which is called in place of the F90-netCDF intrinsic function nf90_get_att in ropp_io/ncdf/ncdf_getatt.f90 and ropp_io/ncdf/ncdf_getvar.f90. There are significant modifications to SUBROUTINE ncdf_getvar_text in this last file. Similarly for SUBROUTINE ncdf_putvar_text in ropp_io/ncdf/ncdf_putvar.f90.

I don't really feel competent to review these changes, but we know from EUMETSAT's extensive experience that they work in practice. And without them, ROPP cannot read new format PPF files: it says

---------------------------------------------------------------------
                  EUMETSAT to ROPP netCDF Converter
---------------------------------------------------------------------

INFO (from eum2ropp):  Reading file /home/h04/idculv/GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN.nc

FATAL ERROR (from ropp_io_read_eumdata):  NetCDF: Attempt to convert between text & numbers

With them it says:

---------------------------------------------------------------------
                  EUMETSAT to ROPP netCDF Converter
---------------------------------------------------------------------

INFO (from eum2ropp):  Reading file /home/h04/idculv/GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN.nc
INFO (from ropp_io_read_eumdata):  Format Version 14.0
... (from eum2ropp):  Ensuring all profiles are in ascending height order.
... (from ropp_io_thin):  Thinning method: Local polynomial regression with z-dependent (sine-squared) variable bandwidth o
INFO (from eum2ropp):  Profile    1 : OC_20200816144104_METC_G032_EUME
... (from eum2ropp):     Latitude/Longitude           : -34.45, 123.58
... (from eum2ropp):     No. of phase/SNR samples     :      0
... (from eum2ropp):     No. of bending angle samples :    247
... (from eum2ropp):     No. of refractivity samples  :      0
... (from eum2ropp):     No. of geophysical samples   :      0
... (from eum2ropp):     No. of surface geo. samples  :      0
... (from eum2ropp):     No. of model coeff. levels   :      0
INFO (from eum2ropp):  Writing /home/h04/idculv/GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN_ropp.nc

The output file looks OK.

And the old test file still works:

---------------------------------------------------------------------
                  EUMETSAT to ROPP netCDF Converter
---------------------------------------------------------------------

INFO (from eum2ropp):  Reading file ../data/eum_test.n4
INFO (from ropp_io_read_eumdata):  Format Version 11.0
... (from eum2ropp):  Ensuring all profiles are in ascending height order.
... (from ropp_io_thin):  Thinning method: Local polynomial regression with default variable bandwidth on 247 impact height
INFO (from eum2ropp):  Profile    1 : OC_20120909000057_META_G015_EUME
... (from eum2ropp):     Latitude/Longitude           : -49.36, 162.36
... (from eum2ropp):     No. of phase/SNR samples     :      0
... (from eum2ropp):     No. of bending angle samples :    247
... (from eum2ropp):     No. of refractivity samples  :      0
... (from eum2ropp):     No. of geophysical samples   :      0
... (from eum2ropp):     No. of surface geo. samples  :      0
... (from eum2ropp):     No. of model coeff. levels   :      0
INFO (from eum2ropp):  Writing temp2.nc

Needs to be tested on all the other test files, but, for now, commit at r6439.

comment:20 by Ian Culverwell, 4 years ago

ropp_io/ncdf/Makefile.am and ropp_io/build/Makefile.am were missing from the original zipfile eumetsat-changes-ropp-v10.0.zip. They're included in the new version (which overwrites the old one).

by Ian Culverwell, 4 years ago

eumetsat-changes-ropp-v10.0.zip

comment:21 by Ian Culverwell, 4 years ago

Some example newer format PPF files, obtained by unpacking some files from ftp://ftp.eumetsat.int/pub/OPS/out/test-data/GRAS_Wave_Optics/metop_{a,b,c}/eps with epsar, are attached for testing purposes.

by Ian Culverwell, 4 years ago

GRAS_1B_M01_20200817073906Z_20200817074016Z_N_T_20200817102018Z_G09_NN.nc

by Ian Culverwell, 4 years ago

GRAS_1B_M01_20200817074017Z_20200817074117Z_N_T_20200817102018Z_G30_NN.nc

by Ian Culverwell, 4 years ago

GRAS_1B_M02_20200816132527Z_20200816133124Z_N_T_20200816162901Z_G07_NN.nc

by Ian Culverwell, 4 years ago

GRAS_1B_M02_20200816133009Z_20200816133500Z_N_T_20200816162901Z_G05_NN.nc

by Ian Culverwell, 4 years ago

GRAS_1B_M03_20200816143505Z_20200816143709Z_N_T_20200816181517Z_G02_NN.nc

by Ian Culverwell, 4 years ago

GRAS_1B_M03_20200816143944Z_20200816144149Z_N_T_20200816181517Z_G28_NN.nc

by Ian Culverwell, 4 years ago

GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN.nc

by Ian Culverwell, 4 years ago

Radio_Occultation_Level_1_Product_Format_Specification.pdf

comment:22 by Ian Culverwell, 4 years ago

Ditto the file format document.

in reply to:  2 comment:23 by Ian Culverwell, 4 years ago

Replying to idculv:

Changes to ropp_io/ropp/ropp_io_types.f90

1) CM says

    REAL(wp), DIMENSION(2)          :: shum        = (/  -20.0_wp,   50.0_wp /)

whereas we reverted this to

    REAL(wp), DIMENSION(2)          :: shum        = (/    0.0_wp,   50.0_wp /)

after the beta version was released. (The valid range of shum is now only relaxed to (-20, 50) g/kg if check_qmin is .FALSE., which, by default, it isn't.) So leave it as it is.

2) CM says

    CHARACTER(len = 160):: thin_method       = "UNKNOWN" ! Profile thinning method

whereas we say

    CHARACTER(len = 80) :: thin_method       = "UNKNOWN" ! Profile thinning method

Why did Chris change this?

... Because in the new files, the attribute

      		string :thinner_method = "Local polynomial regression with z-dependent (sine-squared) variable bandwidth on 247 standard impact height levels" ;

, which is copied into data%thin_method via

      CALL ncdf_getatt(TRIM(ddir)//'thinner_method', data%thin_method)

is more than 80 characters long. Without increasing the length of data%thin_method in ropp_io/ropp/ropp_io_types.f90 the resulting output is truncated to

		:thin_method = "Local polynomial regression with z-dependent (sine-squared) variable bandwidth o" ;

When it's increased to 160, we get

		:thin_method = "Local polynomial regression with z-dependent (sine-squared) variable bandwidth on 247 standard impact height levels [v3.1]" ;

as required.

Change committed at r6440.

in reply to:  7 comment:24 by Ian Culverwell, 4 years ago

Replying to idculv:

5) There's a clash with the check that the closed loop phase measurements are OK.

Chris's code:

        IF (data%FmtVersion < '13.0') THEN
          CALL ncdf_getvar('/quality/cl_snr_ca_ok', readbyte1)
          CALL ncdf_getvar('/quality/cl_snr_p2_ok', readbyte2)
        ELSE
          CALL ncdf_getvar('/quality/snr_l1_ok', readbyte1)
          CALL ncdf_getvar('/quality/snr_l2_ok', readbyte2)
        ENDIF

        IF ((readbyte1 == 0) .AND. (readbyte2 == 0) ) THEN

ROPP10.0:

        CALL ncdf_getvar('/quality/cl_snr_ca_ok', readbyte1)
        CALL ncdf_getvar('/quality/cl_snr_p1_ok', readbyte2)
        CALL ncdf_getvar('/quality/cl_snr_p2_ok', readbyte3)

        IF ((readbyte1 == 0) .AND. (readbyte2 == 0) .AND. (readbyte3 == 0)) THEN

The check on cl_snr_p1_ok has been in the code since at least ROPP-9.1, so why has Chris removed it? Does the new variable snr_l1_ok cover both ca and p1?

To be discussed.

At a telecon on 18/8/2020, Chris explained that cl_snr_p1_ok = cl_snr_ca_ok, so it's fine to remove the p1 check. Probably desirable too, as p1 is never used. Stig's happy with this, and it works OK on old and new files, so commit change, which includes a clause to accommodate the new variables in the new data format, at r6441.

in reply to:  8 comment:25 by Ian Culverwell, 4 years ago

Replying to idculv:

6) external_navbits_applied updated for newer files at r6418.

Should

      IF ( TRIM(ADJUSTL(getlevel1a)) == 'cl+rs' ) THEN
        sampling_mode              = 'raw_sampling'
        IF (data%FmtVersion < '13.0') THEN
          data_available           = 'rs_data_available'
          external_navbits_applied = 'rs_external_navbits_applied'
        ELSE
          data_available           = 'ol_data_available'
          external_navbits_applied = 'external_navbits_ok'
        ENDIF
        ts_olrs = 0.001_wp ! expected (approximate) time in seconds between raw sampling samples
      ENDIF

really be

      IF ( TRIM(ADJUSTL(getlevel1a)) == 'cl+rs' ) THEN
        sampling_mode              = 'raw_sampling'
        IF (data%FmtVersion < '13.0') THEN
          data_available           = 'rs_data_available'
          external_navbits_applied = 'rs_external_navbits_applied'
        ELSE
          data_available           = 'rs_data_available'
          external_navbits_applied = 'external_navbits_ok'
        ENDIF
        ts_olrs = 0.001_wp ! expected (approximate) time in seconds between raw sampling samples
      ENDIF

? Impossible to know without a test file. Ask Chris to confirm/deny.

It's ol_data_available, as in Chris's code. There is no rs_data_available variable in the new format.

in reply to:  14 comment:26 by Ian Culverwell, 4 years ago

Replying to idculv:

12) Untested code to handle the (lev1b) bangles committed at r6426. Why is the version testing written in terms of

      IF (ncdf_isvar(TRIM(ddir)//'bangle_ca')) THEN  !For backward compatibility
        CALL ncdf_getvar(TRIM(ddir)//'bangle_ca',  data%Lev1b%bangle_L1,  &
        units=data%Lev1b%units%bangle)
      ELSE
        CALL ncdf_getvar(TRIM(ddir)//'bangle_l1',  data%Lev1b%bangle_L1,  &
          units=data%Lev1b%units%bangle)
      ENDIF

(etc) rather than

      IF (data%FmtVersion < '13.0') THEN
        CALL ncdf_getvar(TRIM(ddir)//'bangle_ca',  data%Lev1b%bangle_L1,  &
        units=data%Lev1b%units%bangle)
      ELSE
        CALL ncdf_getvar(TRIM(ddir)//'bangle_l1',  data%Lev1b%bangle_L1,  &
          units=data%Lev1b%units%bangle)
      ENDIF

(etc), as it is for all the other changes?

At a telecon on 18/08/2020, Chris confirmed this was a mistake. Reverted to IF (data%FmtVersion < '13.0') version at r6442.

in reply to:  16 comment:27 by Ian Culverwell, 4 years ago

Replying to idculv:

14) Untested changes to the creating of DTpro committed at r6429.

Chris: what are

    WRITE(readstr, *) readreal 

and

  data%FmtVersion       = ThisFmtVer

for?

At a telecon on 18/08/2020 Chris conceded that these are probably mistakes. Deleted them at r6443.

comment:28 by Ian Culverwell, 4 years ago

Stig thinks I might have been overcautious in testing the existence of exphase_ca at r5963. (It wasn't there in his https://trac.romsaf.org/ropp/changeset/5427/ropp_src/branches/dev/Share/dmi_trunk_9.0/ropp_io/ropp/ropp_io_read_ncdf_get.f90.) exphase_ca should certainly be there in any (old format) file, so remove it, and the corresponding check on exphase_1c in the new format files, at r6444.

in reply to:  10 comment:29 by Ian Culverwell, 4 years ago

Replying to idculv:

8) There's an issue with the ol/rs phases and snrs: currently we read in uncorrected ones like i_ca_uncorr, but these are unavailable in newer data files. I think the thing to do is to uncorrect the corrected i_ca and q_ca in the new files by multiplying by the navbits and proceeding as we do now. But I don't know how to combine internal and external navbits. (If the external navbits are missing, they are set equal to the internal ones, so multiplying by both wouldn't have any effect.) Stig also mentioned the possibility of just setting the navbits to zero if I & Q are already corrected, but I think we'll end up with different phase_L1s if we use the corrected navbits. This needs discussing. Meanwhile, implement the incomplete changes at r6420.

At a telecon on 18/08/2020 I think we agreed the following:

  • Use the corrected ones for the newer files, and then set the int. and ext. navbits to zero, which should prevent any later (mis-)corrections being applied.
  • Continue to use uncorrected ones with the older files, for backward compatibility. We hope and expect that in this case the ropp_pp pre-processing will correctly handle the pi phase shifts that result from accumulating the uncorrected Is and Qs.

NUllifying the navbits like this only changes open_loop_lcf in the output file. For example, for input file GRAS_1B_M03_20200816144104Z_20200816144305Z_N_T_20200816181517Z_G32_NN.nc, it changes from

   open_loop_lcf = 
   53, 53, 37, 37, 53, 53, 37, 37, 39, 39, 39, 39, 37, 37, 39, 39, 53, 53, 39, 39, 37, 37, 55, 55, 37, 37, 55, 55, 53, 53, 39, ... ;

to

   open_loop_lcf = 
   37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37, 37 , ... ;

(This is every 10th point of the first 300.) This is a difference of

   open_loop_lcf = 
   -16, -16, 0, 0, -16, -16, 0, 0, -2, -2, -2, -2, 0, 0, -2, -2, -16, -16, -2, -2, 0, 0, -18, -18, 0, 0, -18, -18, -16, -16, -2, ... ;

which makes sense because open_loop_lcf increases by 21=2 if navbit_external = 1, and by 24=16 if navbit_internal = 1, and for this input file,

   navbits_external = 
   -1, -1, -1, -1, -1, -1, -1, -1, 1, 1, 1, 1, -1, -1, 1, 1, -1, -1, 1, 1, -1, -1, 1, 1, -1, -1, 1, 1, -1, -1, 1, ... ;

and

   navbits_internal = 
   1, 1, -1, -1, 1, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 1, 1, -1, -1, -1, -1, 1, 1, -1, -1, 1, 1, 1, 1, -1, ... ;

(Note that all 2 X 2 possible combinations of external and internal navbits occur. Neither is a subset of the other. The (as yet) unanswered question of how to combine them, if we wanted to undo the corrections, therefore remains relevant.)

When the navbits are set to zero, as in the new code, these increments to open_loop_lcf are missing, and so it takes the constant value of 37 = 1 + 4 + 32 = 20 + 22 + 25 = open loop + (external) navbit quality OK (assumed - why?) + alternative (=internal) navbit quality OK (assumed - why?).

The impact of this on the downstream processing is to be determined by other testing later. Meanwhile, change committed at r6445. (Note typo in commit message: I meant "corrected", not "uncorrected".)

in reply to:  12 comment:30 by Ian Culverwell, 4 years ago

Replying to idculv:

10) The raw sampling corrected i_ca and q_ca, needed to calculate the properly averaged SNR, have different names in the new files. (Untested) change committed at r6422. (This includes some cosmetic reformatting.)

NB: Chris says that EUMETSAT have now fixed the problem that this code section was designed to solve. When we know the version where this was fixed, we should wrap the averaging step in an IF ( vn < vn0 ) THEN ... ENDIF construction. Chris: please tell us vn0.

Some crawling through the code by Chris, and some data files from Stig, reveal that vn0 is in fact 4.5.1 of the PPF. Unfortunately this number is not encoded in the netCDF file. All we have is, for example,

    // group attributes:
                                string :processor_name = "YAROS" ;
                                string :processor_version = "1.8" ;
                                string :processing_mode = "NRT" ;
                                string :format_version = "14.0" ;
                                string :source = "GRAS_xxx_00_M03_20200816144200Z_20200816144459Z_N_O_20200816154111Z" ;
                                string :baseline = "" ;
                                string :idb_version = "M03 A/S on v1.3" ;
                                string :processing_centre = "TCE1" ;
                                string :generating_facility = "YAR" ;

in status/processing. There's nothing about the PPF version in there. Chris confirms this, because

Actually, that makes sense - we put information on the actual processor in those fields, and the test data (as well as any 
reprocessed data) is produced  with our reference processor; in operations, it will be the PPF. The confusion become worse 
in the future, because EPS-SG is yet another processor (with independent versioning, of course). At least the Sentinel-6 
/Jason-CS processor is Yaros as well.

I don't know how to handle that in the easiest way. You could separately for the different processors. In this, I would 
simply go for format version number, because that will hopefully be consistent between the various processors. You 
could treat the newer format versions as being ok, and recalculate the SNRs for the older ones.

I agree with that solution, especially as a bit of testing has revealed that Stig's code does very nearly (within 0.02%) the same SNR averaging as EUMETSATs, so it shouldn't make much difference anyway. Therefore a change to test on IF (data%FmtVersion < '13.0') has been tested and committed at r6456.

comment:31 by Ian Culverwell, 4 years ago

For the record, here is the difference between the input open loop snr_ca (as it was then called) and Stig's calculation, based on navbit-corrected raw sampling Is and Qs, i_ca and q_ca}: PPF_445.png We can see the large offset in the EUMETSAT SNR, which arises because the average of the modulus >/= modulus of the average. Stig's calculation correctly averages the real and imaginary parts before calculating the modulus.

At PPF 4.5.1 the error has clearly been corrected: PPF_451.png Fractional differences (not shown) are of the order of 0.02%, which is considered acceptable.

by Ian Culverwell, 4 years ago

Attachment: PPF_445.png added

PPF_445.png

by Ian Culverwell, 4 years ago

Attachment: PPF_451.png added

PPF_451.png

comment:32 by Ian Culverwell, 4 years ago

Resolution: fixed
Status: newclosed

All done. Closing ticket.

Note: See TracTickets for help on using tickets.