Dangerous code in ProCmnFillType template class

Issues related to runtime execution of algorithms in ADL
Post Reply
JimBiard
Posts: 26
Joined: Thu Jun 30, 2011 8:09 am
Location: NCDC

Dangerous code in ProCmnFillType template class

Post by JimBiard »

Hi.

There's no current ADL sub-forum that is applicable for this post, because the potential problem can affect code across the board. So, I decided to post it here.

The ProCmnFillType template class has a method named isFill. This method allows you to test for fill values for any basic type (Int8, UInt8, ..., Float64). The problem is, the fill values for signed types are not at the edge of the possible valid range, yet the test for fill is (this is genericized - replace (TYPE) with the appropriate type name)

valueToCheck < (TYPE)_FILL_TEST

As a result, any signed type values that are more negative than the FILL_TEST boundary are reported as being fill, even though the value is not one of the defined fill values. This can be of particular concern for 14-bit signed integer values that have been converted to 16-bit signed integers. It sure seems like the isFill methods should be changed to avoid marking non-fill signed values as fill.

I don't know enough of the internals of all the IDPS algorithms to know if there are any places where this is causing problems, but I got bit by it in code I am writing, so I thought I would document it by writing this post.

Grace and peace,

Jim Biard
Research Scholar
Cooperative Institute for Climate and Satellites
Remote Sensing and Applications Division (RSAD)
National Climatic Data Center
151 Patton Ave.
Asheville, NC 28801-5001
houchin
Posts: 128
Joined: Mon Jan 10, 2011 6:20 am

Re: Dangerous code in ProCmnFillType template class

Post by houchin »

I'm not sure what the justification was for placing the fill values where they were placed, but I expect there's no chance on changing that.

Ultimately that means to me though that you can't have valid signed data where the range of that data includes the fill values; valid data can't be fill. Given that, I I would think that your data should indeed have been marked as fill, as was erroneous. If the isFill method was modified to check for only the specific fill values, I would think that intermittent bugs would be even harder to find, as you'd be seeing sporadic fill in what should be valid data where the the value of a specific point just happened to match a fill value.
Scott Houchin, Senior Engineering Specialist, The Aerospace Corporation
15049 Conference Center Dr CH3/310, Chantilly, VA 20151; 571-307-3914; scott.houchin@aero.org
JimBiard
Posts: 26
Joined: Thu Jun 30, 2011 8:09 am
Location: NCDC

Re: Dangerous code in ProCmnFillType template class

Post by JimBiard »

There's no good solution to the more general problem of the fill values falling within valid data ranges, I agree. But I was floored when I discovered that the isFill test for signed types didn't limit itself to the range of actual fill values.
Research Scholar
Cooperative Institute for Climate and Satellites
Remote Sensing and Applications Division (RSAD)
National Climatic Data Center
151 Patton Ave.
Asheville, NC 28801-5001
JimBiard
Posts: 26
Joined: Thu Jun 30, 2011 8:09 am
Location: NCDC

Re: Dangerous code in ProCmnFillType template class

Post by JimBiard »

The problem I encountered arose because of a value that started as a 14-bit signed integer stored in a Int16. It always reads as positive until sign extension is done on it, so before that point all negative values are fill indicators. Once you convert it to a 16-bit signed integer by extending the value of bit 13 (from zero) to the top of the word, the fill values start falling inside the range of valid values. There are quite a few VIIRS engineering data values that fit in this category. When the VIIRS verified RDR is constructed, many 14-bit signed values are converted this way. The values outside the -999 - -992 range are completely valid. The valid raw ranges for these values are stated as -8192 - 8192. Some of them do, in fact, run below -991.

The big point here is to be quite careful about trusting the result of the isFill method for signed types.
Research Scholar
Cooperative Institute for Climate and Satellites
Remote Sensing and Applications Division (RSAD)
National Climatic Data Center
151 Patton Ave.
Asheville, NC 28801-5001
houchin
Posts: 128
Joined: Mon Jan 10, 2011 6:20 am

Re: Dangerous code in ProCmnFillType template class

Post by houchin »

How can you even be using the "standard" fill values for those parameters, unless the real data range isn't really the numerical limits for 14-bit signed values?
Scott Houchin, Senior Engineering Specialist, The Aerospace Corporation
15049 Conference Center Dr CH3/310, Chantilly, VA 20151; 571-307-3914; scott.houchin@aero.org
JimBiard
Posts: 26
Joined: Thu Jun 30, 2011 8:09 am
Location: NCDC

Re: Dangerous code in ProCmnFillType template class

Post by JimBiard »

It's not me, it's in the IDPS software. Ask them. ;)
Research Scholar
Cooperative Institute for Climate and Satellites
Remote Sensing and Applications Division (RSAD)
National Climatic Data Center
151 Patton Ave.
Asheville, NC 28801-5001
bhenders
Posts: 72
Joined: Wed Jan 05, 2011 9:27 am
Location: Omaha, NE

Re: Dangerous code in ProCmnFillType template class

Post by bhenders »

All,

I'm not sure I want to jump into this topic, but probably need to give you some sort of response. I was sort of happy leaving it as "The big point here is to be quite careful about trusting the result of the isFill method for signed types." The actual fill values were chosen long before I arrived on the program and probably were directed from data working groups. The issue of data overlap with the fill values has also come up previously, such that I believe the NHF EDR actually has to use defined fill values of -9999.x instead of -999.x so the valid data range does not overlap with the IDPS normally defined fill values.

From what you've typed below you have a range of data from -8192 to 8192 which is definitely an overlap with the -99X fill values. However, what isn't clear to me is this existing IDPS code or code that you've written to use the ProCmnFillType class. I'm guessing the latter. I would say that using the fill type class to check packet or verified RDR data does not make any sense, since the data has not been conformed to fit within a non overlapping fill range, but I suspect that is the conclusion you've come to and are alerting others of this fact. Basically, this class's value is to check for fill for data that has been produced by IDPS to be in a conforming range with the IDPS fill values for that data type. Using it on packet data of 14 bit signed values to int16 does not provide value since the packet data itself does not conform to the IDPS fill values.

Reply only if we need to discuss the topic further, but I felt some reply was needed. Again this class comes from IDPS and isn't specific to ADL at all, so it's origination and use is driven by IDPS.

Thanks,

Bryan Henderson
Raytheon Company
Post Reply