Merge pull request #488 from lsst/tickets/DM-21355

DM-21355: Try to prevent NaN values turning up in FITS headers
lsst · Sep 30, 2019 · f57f488 · f57f488
2 parents d2166b6 + 75ba42d
commit f57f488
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 7 deletions.
diff --git a/src/fits.cc b/src/fits.cc
@@ -66,10 +66,28 @@ std::string makeLimitedFitsHeaderImpl(std::vector<std::string> const &paramNames
         } else if (type == typeid(int)) {
             out += (boost::format("%20d") % metadata.get<int>(name)).str();
         } else if (type == typeid(double)) {
-            // use G because FITS wants uppercase E for exponents
-            out += (boost::format("%#20.17G") % metadata.get<double>(name)).str();
+            double value = metadata.get<double>(name);
+            if (!std::isnan(value)) {
+                // use G because FITS wants uppercase E for exponents
+                out += (boost::format("%#20.17G") % value).str();
+            } else {
+                LOGLS_WARN("afw.fits",
+                           boost::format("In %s, found NaN in metadata item '%s'") %
+                                         BOOST_CURRENT_FUNCTION % name);
+                // Convert it to FITS undefined
+                out += " ";
+            }
         } else if (type == typeid(float)) {
-            out += (boost::format("%#20.15G") % metadata.get<float>(name)).str();
+            float value = metadata.get<float>(name);
+            if (!std::isnan(value)) {
+                out += (boost::format("%#20.15G") % value).str();
+            } else {
+                LOGLS_WARN("afw.fits",
+                           boost::format("In %s, found NaN in metadata item '%s'") %
+                                         BOOST_CURRENT_FUNCTION % name);
+                // Convert it to FITS undefined
+                out += " ";
+            }
         } else if (type == typeid(std::nullptr_t)) {
             out += " ";
         } else if (type == typeid(std::string)) {

diff --git a/src/geom/detail/frameSetUtils.cc b/src/geom/detail/frameSetUtils.cc
@@ -27,6 +27,9 @@
 #include <set>
 #include <sstream>
 #include <vector>
+#include <cmath>
+
+#include "boost/format.hpp"
 
 #include "astshim.h"
 
@@ -40,6 +43,7 @@
 #include "lsst/daf/base/PropertyList.h"
 #include "lsst/daf/base/PropertySet.h"
 #include "lsst/pex/exceptions.h"
+#include "lsst/log/Log.h"
 
 namespace lsst {
 namespace afw {
@@ -347,10 +351,21 @@ ast::FitsChan getFitsChanFromPropertyList(daf::base::PropertySet& metadata,
                 fc.setFitsI(name, static_cast<int>(metadata.get<std::uint8_t>(name)));
             } else if (type == typeid(int)) {
                 fc.setFitsI(name, metadata.get<int>(name));
-            } else if (type == typeid(double)) {
-                fc.setFitsF(name, metadata.get<double>(name));
-            } else if (type == typeid(float)) {
-                fc.setFitsF(name, static_cast<double>(metadata.get<float>(name)));
+            } else if (type == typeid(double) || type == typeid(float)) {
+                double value;
+                if (type == typeid(double)) {
+                    value = metadata.get<double>(name);
+                } else {
+                    value = static_cast<double>(metadata.get<float>(name));
+                }
+                // NaN is not allowed in a FitsChan (or in FITS)
+                if (!std::isnan(value)) {
+                    fc.setFitsF(name, value);
+                } else {
+                    // Treat it like an undefined value but warn about it
+                    LOGLS_WARN("afw.geom.frameSetUtils",
+                               boost::format("Found NaN in metadata item '%s'") % name);
+                }
             } else if (type == typeid(std::string)) {
                 std::string str = metadata.get<std::string>(name);
                 // No support for long strings yet so skip those

diff --git a/tests/test_frameSetUtils.py b/tests/test_frameSetUtils.py
@@ -73,6 +73,18 @@ def getCrpix(self, metadata):
             metadata.getScalar("CRPIX2") + metadata.getScalar("CRVAL2A") - 1,
         )
 
+    def testIgnoreNan(self):
+        """Test that NaN in a property list does not crash the WCS extraction.
+        """
+        metadata = self.makeMetadata()
+        metadata["ISNAN"] = float("NaN")
+
+        # This will issue a warning from C++ but that warning can not be
+        # redirected to python logging machinery.
+        # This code causes a SIGABRT without DM-21355 implemented
+        frameSet1 = readFitsWcs(metadata, strip=False)
+        self.assertEqual(type(frameSet1), ast.FrameSet)
+
     def testReadFitsWcsStripMetadata(self):
         metadata = self.makeMetadata()
         nKeys = len(metadata.toList())

diff --git a/tests/test_makeLimitedFitsHeader.py b/tests/test_makeLimitedFitsHeader.py
@@ -101,6 +101,7 @@ def testBasics(self):
             ("LONGSTR", "skip this item because the formatted value "
                 "is too long: longer than 80 characters "),
             ("ASTRING1", "value for string"),
+            ("ANAN", float("NaN"))
         ]
         for name, value in dataList:
             metadata.set(name, value)
@@ -117,6 +118,7 @@ def testBasics(self):
             "LONGFLT = 0.0089626337538440005",
             "ANUNDEF =",
             "ASTRING1= 'value for string'",
+            "ANAN    =",
         ]
         expectedHeader = "".join("%-80s" % val for val in expectedLines)