BUFR delayed replication descriptor

[michaeldiener]

Versions impacted by the bug

bufr 5.5.3 and later (probably earlier, too)

What went wrong?

I believe the handling of delayed replication descriptors is incomplete. The class and method that I mean is ucar.nc2.iosp.bufr.DataDescriptorTreeConstructor method private List<DataDescriptor> replicate(List<DataDescriptor> keys).

I would create a patch if I would feel more confident about the BUFR specification. Ideally an expert could look my findings here over.

Let me copy + paste the relevant part of the method here:

      DataDescriptor dk = dkIter.next();
      if (dk.f == 1) {
        dk.subKeys = new ArrayList<>();
        dk.replication = dk.y; // replication count

        if (dk.replication == 0) { // delayed replication
          root.isVarLength = true; // variable sized data == deferred replication == sequence data

          // the next one is the replication count size : does not count in field count (x)
          DataDescriptor replication = dkIter.next();
          if (replication.y == 0)
            dk.replicationCountSize = 1; // ??
          else if (replication.y == 1)
            dk.replicationCountSize = 8;
          else if (replication.y == 2)
            dk.replicationCountSize = 16;
          else if (replication.y == 11)
            dk.repetitionCountSize = 8;
          else if (replication.y == 12)
            dk.repetitionCountSize = 16;
          else
            log.error("Unknown replication type= " + replication);
        }

        // transfer to the subKey list
        for (int j = 0; j < dk.x && dkIter.hasNext(); j++) {
          dk.subKeys.add(dkIter.next());
        }

        // recurse
        dk.subKeys = replicate(dk.subKeys);

      }

1. For replication.x == 31 there should be specific handling. I suggest the following:

   ...
         // the next one is the replication count size : does not count in field count (x)
         DataDescriptor replication = dkIter.next();
   
         if (replication.x == 31)
           dk.replicationCountSize = replication.bitWidth;
         else if (replication.y == 0)
   ...

   This is relevant in the case of BUFR radar files from Meteo France. I will try to attach an example here for reference (taken from their new Open Data site). If attaching does not work, I've uploaded it to dropbox: https://www.dropbox.com/scl/fi/vrl5ym1fyg7l7bqzcj1m7/NOUVELLE-CALEDONIE.bufr?rlkey=4r0yohp5472qi0ptifx81k9c9&dl=0 (you will need the Meteo France BUFR tables as well, I try to attach them, too)

The file contains for example the following sequence of descriptors: "1-1-0: Replication" "0-31-192: Facteur super elargi de repetition differe du descripteur units=Numeric scale=0 refVal=0 nbits=32"

Now the old code does not handle a replicator.y == 192, the new code does correctly. Without the proposed change the some of the BUFR file cannot be read (the actual radar data).

2. Not sure why there is a differentiation between replicationCountSize and repetitionCountSize. replicationCountSize is used in the code and is relevant while repetitionCountSize is never used (unless you count the toString method). Also I'm unsure if hard coding those values is correct or a kind of shortcut to not use the actual BUFR tables. This is something somebody more experienced with BUFR should find out.

Relevant stack trace

No response

Relevant log messages

No response

If you have an example file that you can share, please attach it to this issue.

If so, may we include it in our test datasets to help ensure the bug does not return once fixed? Note: the test datasets are publicly accessible without restriction.

Yes

Code of Conduct

• [X] I agree to follow the UCAR/Unidata Code of Conduct

[michaeldiener]

Example file: NOUVELLE-CALEDONIE.bufr.gz Needed BUFR tables including a tablelookup.csv that needs to be loaded via ucar.nc2.iosp.bufr.tables.BufrTables.addLookupFile: tables_bufr_281x.zip

[tdrwenski]

Hi @michaeldiener, thanks for reporting this issue. We rely on community support for BUFR, so if your suggested change doesn't break the existing tests and it fixes things for your case, I would encourage you to submit a pull request for this! We would be happy to help you with this if you have questions.

[michaeldiener]

Hi @tdrwenski, alright, will do, just have to find the time and so it might take a while until I get to. Thanks!

[michaeldiener]

Closing this issue as PR was merged