Unexpected result when scaling var with odd attributes

[rschmunk]

In the changes this summer in how NJ 5.0 handles scaling and adding offsets, there's also been a change in the result of reporting values from oddly defined variables. A user queried me about a dataset that has a variable defined by

float BC(time=1, lat=1800, lon=3600);
  :_FillValue = -9999.0f; // float
  :long_name = "Black Carbon";
  :units = "kg/m2/s";
  :add_offset = 0L; // long
  :scale_factor = 1L; // long

Note that the scaling factor and offset would, at first glance, suggest that the float values of BC would not be altered. However, it turns out the values are being cast to long ints to match the attribute types. Since the actual data values are quite small (in the vicinity of 1e-10), the result is that any "good" values in the variable array are being reported as equaling 0L.

So I'm not sure if this counts as a bug in the NJ library code, or a bug in the user's dataset. Thoughts?

[cwardgar]

Good catch; this is a bug. Luckily the fix is simple. In EnhanceScaleMissingUnsignedImpl.rank(), FLOAT should have a greater rank than LONG and ULONG. Also, the description of the method should be changed. Right now, it says:

Returns a distinct integer for each of the {@link DataType#isNumeric() numeric} data types
that can be used to (roughly) order them by {@link DataType#getSize() size}.

However, thinking about it in terms of the size of the data type (e.g. 4 bytes for float, 8 bytes for long) is wrong. With the above change, we're ordering based on the range of the data type. The range of LONG is -9,223,372,036,854,775,808 to 9,223,372,036,854,775,807. The range of FLOAT is ±3.40282347E+38F. Therefore, FLOAT should have a greater rank.

[rschmunk]

@cwardgar, If it looks that easy, any hope of a commit in the next week or so?

[lesserwhirls]

Unfortunately for us, @cwardgar is no longer at Unidata (thank you for the pointer, @cwardgar!). I'll try to look at it today and hopefully get something to you.

[lesserwhirls]

I just made a PR and am running it against our full test suite. If all goes well, we'll have a new artifact for in a few hours. Thanks @cwardgar !

[lesserwhirls]

Let's see if that does it - just published new artifacts.

[rschmunk]

Thanks, @lesserwhirls. I'll check whether this solves the sample (i.e., complaint) case Monday afternoon.

[rschmunk]

@lesserwhirls, Looks like this fixes the examples I was dealing with. Thanks.