[Insight-developers] Debian Sid _nrrdRead: sanity check FAILED

[Gordon L. Kindlmann]

Thanks for the further information on this.

Based on this, I've removed he signaling NaN checks from Teem, and from the NrrdIO on Sourceforge that is created automatically from Teem.   Here's what the change looked like:

===================================================================
--- sane.c	(revision 5170)
+++ sane.c	(working copy)
@@ -101,25 +101,25 @@
 
   if (!( airFP_QNAN == airFPClass_f(AIR_NAN)
          && airFP_QNAN == airFPClass_f(AIR_QNAN)
-/*
-  Exclude the following platforms from the airFP_SNAN test.
-
- 1) APPLE builds due to a cross-compilation problem, and
- 2) Visual Studio builds for version newer than 2005 (not included)
- when building in 32bits. */
-
-#if defined(__APPLE__) || ( defined(_MSC_VER) && _MSC_VER >= 1400 ) 
-         /* don't compare airFP_SNAN */
-#else
-         && airFP_SNAN == airFPClass_f(AIR_SNAN) 
-#endif
+         /*
+           As of July 4 2012 GLK decides that the signalling NaN tests are
+           more trouble than they're worth: the signal-ness of the NaN is not
+           preserved in double-float conversion for some platforms (so
+           airFP_SNAN == airFPClass_d(AIR_SNAN) has never been enforced), and
+           there are more platforms for which (apparently) passing AIR_SNAN to
+           airFPClass_d changes it to a quiet NaN, which defeats the purpose
+           of the test.  To summarize, given that:
+           ** AIR_NAN and AIR_QNAN are checked here to be quiet NaN, after
+              casting to both float and double,
+           ** quiet NaN "hi bit" is tested above, and that
+           ** quiet and signalling NaN are mutually exclusive,
+           skipping the signalling NaN tests is unlikely to undermine knowing
+           the correctness of the compile-time representation of NaNs.  So the
+           following line is now commented out for all platforms.
+         */
+         /* && airFP_SNAN == airFPClass_f(AIR_SNAN) */
          && airFP_QNAN == airFPClass_d(AIR_NAN)
          && airFP_QNAN == airFPClass_d(AIR_QNAN) )) {
-    /* we don't bother checking for 
-       airFP_SNAN == airFPClass_d(AIR_SNAN) because
-       on some platforms the signal-ness of the NaN
-       is not preserved in double-float conversion */
-
     return airInsane_AIR_NAN;
   }
   if (!(airFP_QNAN == airFPClass_f(nanF)



Now the challenge is to re-synchronize ITK's NrrdIO with Sourceforge's NrrdIO so that everyone can benefit from this simplification.  This process has been done before:

http://www.itk.org/Wiki/Proposals:Updating_Nrrd_library_2009

Is there anyone who is in a position to do this again?

Gordon


On Jul 3, 2012, at 9:38 PM, Steve M. Robbins wrote:

> On Tue, Jul 03, 2012 at 09:11:58AM -0400, Bradley Lowekamp wrote:
>> Hi Steve,
>> 
>> I got a bump on by gcc on my amd64 Debian Sid to version 4.7.1-2.
>> 
>> I am curious if this particular patch is still needed or if they fixed the bug in the compiler.
>> 
>> Could you try the build on a i386 system?
>> 
>> If you just revert the commit:
>> 
>> git revert 21da36bc995a0628006b486ac7c66003f6cf07d8
>> 
>> you should be good to give it a compile and test, after that command.
>> 
> 
> I'm sorry to report that it still fails:
> 
> 
> 1: itk::ERROR: NrrdImageIO(0xa9cc4d8): ReadImageInformation: Error reading /home/steve/Packages/insighttoolkit/upstream/ITK/build32/Testing/Temporary/testNrrd.nrrd:
> 1: [nrrd] nrrdLoad: trouble reading "/home/steve/Packages/insighttoolkit/upstream/ITK/build32/Testing/Temporary/testNrrd.nrrd"
> 1: [nrrd] nrrdRead: trouble
> 1: [nrrd] _nrrdRead: sanity check FAILED: have to fix and re-compile
> 1: [nrrd] nrrdSanity: airSanity() failed: airFPClass(AIR_QNAN,AIR_SNAN) wrong
> 
> 
> -Steve