Repository: CMB-S4/spt3g_software (language: python)

Issue:
G3MapVectorInt and G3MapInt should use int64
Currently these use int32, which is inconsistent with G3VectorInt.


Discussion / hints:

Fix patch:
diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt
index 70cb51b8..1e11241b 100644
--- a/core/CMakeLists.txt
+++ b/core/CMakeLists.txt
@@ -25,6 +25,7 @@ add_spt3g_library(core SHARED
 	src/crc32.c ${CORE_EXTRA_SRCS}
 	src/G3NetworkSender.cxx src/G3SyslogLogger.cxx
 	src/G3PipelineInfo.cxx src/G3Quat.cxx
+	src/int_storage.cxx
 )
 
 # Link dependencies
diff --git a/core/include/core/G3Map.h b/core/include/core/G3Map.h
index 128695ec..830e82d0 100644
--- a/core/include/core/G3Map.h
+++ b/core/include/core/G3Map.h
@@ -27,6 +27,9 @@ class G3Map : public G3FrameObject, public std::map<Key, Value> {
 		    cereal::base_class<std::map<Key, Value> >(this));
 	}
 
+	template <class A> void load(A &ar, unsigned v);
+	template <class A> void save(A &ar, unsigned v) const;
+
 	std::string Summary() const
 	{
 		if (this->size() < 5)
@@ -72,16 +75,25 @@ G3MAP_OF(std::string, double, G3MapDouble);
 G3MAP_OF(std::string, G3MapDouble, G3MapMapDouble);
 G3MAP_OF(std::string, std::vector<double>, G3MapVectorDouble);
 G3MAP_OF(std::string, std::vector<bool>, G3MapVectorBool);
-G3MAP_OF(std::string, std::vector<int32_t>, G3MapVectorInt);
 G3MAP_OF(std::string, std::vector<std::string>, G3MapVectorString);
 G3MAP_OF(std::string, G3VectorVectorString, G3MapVectorVectorString);
 G3MAP_OF(std::string, std::vector<std::complex<double> >, G3MapVectorComplexDouble);
 G3MAP_OF(std::string, G3VectorTime, G3MapVectorTime);
-G3MAP_OF(std::string, int32_t, G3MapInt);
 G3MAP_OF(std::string, std::string, G3MapString);
 G3MAP_OF(std::string, quat, G3MapQuat);
 G3MAP_OF(std::string, G3VectorQuat, G3MapVectorQuat);
 
+#define G3MAP_SPLIT(key, value, name, version) \
+typedef G3Map< key, value > name; \
+namespace cereal { \
+	template <class A> struct specialize<A, name, cereal::specialization::member_load_save> {}; \
+} \
+G3_POINTERS(name); \
+G3_SERIALIZABLE(name, version);
+
+G3MAP_SPLIT(std::string, std::vector<int64_t>, G3MapVectorInt, 2);
+G3MAP_SPLIT(std::string, int64_t, G3MapInt, 2);
+
 namespace cereal {
         template <class A> struct specialize<A, G3MapFrameObject, cereal::specialization::member_load_save> {};
 }
diff --git a/core/src/G3Map.cxx b/core/src/G3Map.cxx
index aeff5d26..ef0f5f02 100644
--- a/core/src/G3Map.cxx
+++ b/core/src/G3Map.cxx
@@ -3,6 +3,152 @@
 #include <pybindings.h>
 #include <container_pybindings.h>
 #include <serialization.h>
+#include "int_storage.h"
+
+/* Special load/save for int64_t, using the same encoding at G3VectorInt */
+
+template <>
+template <class A>
+void G3Map<std::string, int64_t>::load(A &ar, const unsigned v)
+{
+	G3_CHECK_VERSION(v);
+
+	ar & cereal::make_nvp("G3FrameObject",
+	    cereal::base_class<G3FrameObject>(this));
+	int store_bits = 32;
+	if (v >= 2)
+		ar & cereal::make_nvp("store_bits", store_bits);
+
+	switch(store_bits) {
+	case 64:
+		ar & cereal::make_nvp("map",
+		    cereal::base_class<std::map<std::string, int64_t> >(this));
+		break;
+	case 32:
+		load_as<A, int32_t>(ar, *this);
+		break;
+	case 16:
+		load_as<A, int16_t>(ar, *this);
+		break;
+	case 8:
+		load_as<A, int8_t>(ar, *this);
+		break;
+	}
+}
+
+template <>
+template <class A>
+void G3Map<std::string, int64_t>::save(A &ar, const unsigned v) const
+{
+	// v == 2
+	ar & cereal::make_nvp("G3FrameObject",
+	    cereal::base_class<G3FrameObject>(this));
+	// Count the interesting bits, and convert to nearest power of 2.
+	int sig_bits = bit_count(*this);
+	int store_bits = 8;
+	while (store_bits < sig_bits)
+		store_bits *= 2;
+	ar & cereal::make_nvp("store_bits", store_bits);
+	switch(store_bits) {
+	case 8:
+		save_as<A, int8_t>(ar, *this);
+		break;
+	case 16:
+		save_as<A, int16_t>(ar, *this);
+		break;
+	case 32:
+		save_as<A, int32_t>(ar, *this);
+		break;
+	default:
+		ar & cereal::make_nvp("map",
+		    cereal::base_class<std::map<std::string, int64_t> >(this));
+	}
+}
+
+template <>
+template <class A>
+void G3Map<std::string, std::vector<int64_t> >::load(A &ar, const unsigned v)
+{
+	G3_CHECK_VERSION(v);
+
+	ar & cereal::make_nvp("G3FrameObject",
+	    cereal::base_class<G3FrameObject>(this));
+
+	if (v == 1) {
+		std::map<std::string, std::vector<int32_t> > temp;
+		ar & cereal::make_nvp("map", temp);
+		for (auto const &i: temp) {
+			std::vector<int64_t> v(i.second.begin(), i.second.end());
+			(*this)[i.first] = v;
+		}
+
+		return;
+	}
+
+	uint32_t len;
+	ar & cereal::make_nvp("len", len);
+
+	for (uint32_t i = 0; i < len; i++) {
+		std::pair<std::string, std::vector<int64_t> > item;
+		ar & cereal::make_nvp("key", item.first);
+		int store_bits;
+		ar & cereal::make_nvp("store_bits", store_bits);
+
+		switch(store_bits) {
+		case 64:
+			ar & cereal::make_nvp("vector", item.second);
+			break;
+		case 32:
+			load_as<A, int32_t>(ar, item.second);
+			break;
+		case 16:
+			load_as<A, int16_t>(ar, item.second);
+			break;
+		case 8:
+			load_as<A, int8_t>(ar, item.second);
+			break;
+		}
+
+		this->insert(item);
+	}
+}
+
+template <>
+template <class A>
+void G3Map<std::string, std::vector<int64_t> >::save(A &ar, const unsigned v) const
+{
+	// v == 2
+	ar & cereal::make_nvp("G3FrameObject",
+	    cereal::base_class<G3FrameObject>(this));
+
+	uint32_t len = size();
+	ar & cereal::make_nvp("len", len);
+
+	for (auto const &i: *this) {
+		ar & cereal::make_nvp("key", i.first);
+
+		// Count the interesting bits, and convert to nearest power of 2.
+		int sig_bits = bit_count(i.second);
+		int store_bits = 8;
+		while (store_bits < sig_bits)
+			store_bits *= 2;
+		ar & cereal::make_nvp("store_bits", store_bits);
+
+		switch(store_bits) {
+		case 8:
+			save_as<A, int8_t>(ar, i.second);
+			break;
+		case 16:
+			save_as<A, int16_t>(ar, i.second);
+			break;
+		case 32:
+			save_as<A, int32_t>(ar, i.second);
+			break;
+		default:
+			ar & cereal::make_nvp("vector", i.second);
+		}
+	}
+}
 
 template <class A> void G3MapFrameObject::save(A &ar, const unsigned v) const
 {
@@ -70,13 +216,11 @@ std::string G3MapFrameObject::Description() const
 	return s.str();
 }
 
-G3_SERIALIZABLE_CODE(G3MapInt);
 G3_SERIALIZABLE_CODE(G3MapDouble);
 G3_SERIALIZABLE_CODE(G3MapMapDouble);
 G3_SERIALIZABLE_CODE(G3MapString);
 G3_SERIALIZABLE_CODE(G3MapQuat);
 G3_SERIALIZABLE_CODE(G3MapVectorBool);
-G3_SERIALIZABLE_CODE(G3MapVectorInt);
 G3_SERIALIZABLE_CODE(G3MapVectorDouble);
 G3_SERIALIZABLE_CODE(G3MapVectorString);
 G3_SERIALIZABLE_CODE(G3MapVectorVectorString);
@@ -84,6 +228,9 @@ G3_SERIALIZABLE_CODE(G3MapVectorComplexDouble);
 G3_SERIALIZABLE_CODE(G3MapVectorTime);
 G3_SERIALIZABLE_CODE(G3MapVectorQuat);
 
+G3_SPLIT_SERIALIZABLE_CODE(G3MapInt);
+G3_SPLIT_SERIALIZABLE_CODE(G3MapVectorInt);
+
 G3_SPLIT_SERIALIZABLE_CODE(G3MapFrameObject);
 
 PYBINDINGS("core") {
diff --git a/core/src/G3Vector.cxx b/core/src/G3Vector.cxx
index 3b8fd584..1d95316a 100644
--- a/core/src/G3Vector.cxx
+++ b/core/src/G3Vector.cxx
@@ -3,6 +3,7 @@
 #include <container_pybindings.h>
 #include <G3Vector.h>
 #include <complex>
+#include "int_storage.h"
 
 G3_SPLIT_SERIALIZABLE_CODE(G3VectorInt);
 G3_SERIALIZABLE_CODE(G3VectorBool);
@@ -16,41 +17,6 @@ G3_SERIALIZABLE_CODE(G3VectorTime);
 
 /* Special load/save for int64_t. */
 
-static
-int bit_count(std::vector<int64_t> const &d) {
-	// Returns the smallest number N such that all ints in the
-	// vector could be safely expressed as intN_t.  Assumes two's
-	// complement integers.  Return value will be between 1 and
-	// 64.
-	uint64_t mask = 0;
-	for (auto c: d) {
-		if (c < 0)
-			mask |= ~c;
-		else
-			mask |= c;
-	}
-	for (int i=1; i<64; i++) {
-		if (mask == 0)
-			return i;
-		mask >>= 1;
-	}
-	return 64;
-}
-
-template <class A, typename FROM_TYPE, typename TO_TYPE>
-void load_as(A &ar, std::vector<TO_TYPE> &dest) {
-	std::vector<FROM_TYPE> temp;
-	ar & cereal::make_nvp("vector", temp);
-	dest.resize(temp.size());
-	std::copy(temp.begin(), temp.end(), dest.begin());
-}
-
-template <class A, typename FROM_TYPE, typename TO_TYPE>
-void save_as(A &ar, const std::vector<FROM_TYPE> &src) {
-	std::vector<TO_TYPE> temp(src.begin(), src.end());
-	ar & cereal::make_nvp("vector", temp);
-}
-
 template <>
 template <class A>
 void G3Vector<int64_t>::load(A &ar, const unsigned v)
@@ -69,13 +35,13 @@ void G3Vector<int64_t>::load(A &ar, const unsigned v)
 				      cereal::base_class<std::vector<int64_t> >(this));
 		break;
 	case 32:
-		load_as<A, int32_t, int64_t>(ar, *this);
+		load_as<A, int32_t>(ar, *this);
 		break;
 	case 16:
-		load_as<A, int16_t, int64_t>(ar, *this);
+		load_as<A, int16_t>(ar, *this);
 		break;
 	case 8:
-		load_as<A, int8_t, int64_t>(ar, *this);
+		load_as<A, int8_t>(ar, *this);
 		break;
 	}
 }
@@ -95,13 +61,13 @@ void G3Vector<int64_t>::save(A &ar, const unsigned v) const
 	ar & cereal::make_nvp("store_bits", store_bits);
 	switch(store_bits) {
 	case 8:
-		save_as<A, int64_t, int8_t>(ar, *this);
+		save_as<A, int8_t>(ar, *this);
 		break;
 	case 16:
-		save_as<A, int64_t, int16_t>(ar, *this);
+		save_as<A, int16_t>(ar, *this);
 		break;
 	case 32:
-		save_as<A, int64_t, int32_t>(ar, *this);
+		save_as<A, int32_t>(ar, *this);
 		break;
 	default:
 		ar & cereal::make_nvp("vector",
diff --git a/core/src/int_storage.cxx b/core/src/int_storage.cxx
new file mode 100644
index 00000000..1d64cab9
--- /dev/null
+++ b/core/src/int_storage.cxx
@@ -0,0 +1,55 @@
+#include "int_storage.h"
+
+int bit_count(const std::vector<int64_t> &d) {
+	// Returns the smallest number N such that all ints in the
+	// vector could be safely expressed as intN_t.  Assumes two's
+	// complement integers.  Return value will be between 1 and
+	// 64.
+	uint64_t mask = 0;
+	for (auto c: d) {
+		if (c < 0)
+			mask |= ~c;
+		else
+			mask |= c;
+	}
+	for (int i=1; i<64; i++) {
+		if (mask == 0)
+			return i;
+		mask >>= 1;
+	}
+	return 64;
+}
+
+int bit_count(const std::map<std::string, int64_t> &d) {
+	// Returns the smallest number N such that all ints in the
+	// map could be safely expressed as intN_t.  Assumes two's
+	// complement integers.  Return value will be between 1 and
+	// 64.
+	uint64_t mask = 0;
+	for (auto c: d) {
+		if (c.second < 0)
+			mask |= ~c.second;
+		else
+			mask |= c.second;
+	}
+	for (int i=1; i<64; i++) {
+		if (mask == 0)
+			return i;
+		mask >>= 1;
+	}
+	return 64;
+}
+
+#define INT_SERIALIZABLE_CODE(inttype) \
+template <typename inttype> \
+void load_as(cereal::PortableBinaryInputArchive &, std::vector<int64_t> &dest); \
+template <typename inttype> \
+void load_as(cereal::PortableBinaryInputArchive &, std::map<std::string, int64_t> &dest); \
+template <typename inttype> \
+void save_as(cereal::PortableBinaryOutputArchive &, const std::vector<int64_t> &src); \
+template <typename inttype> \
+void save_as(cereal::PortableBinaryOutputArchive &, const std::map<std::string, int64_t> &src)
+
+INT_SERIALIZABLE_CODE(int8_t);
+INT_SERIALIZABLE_CODE(int16_t);
+INT_SERIALIZABLE_CODE(int32_t);
diff --git a/core/src/int_storage.h b/core/src/int_storage.h
new file mode 100644
index 00000000..f517fd10
--- /dev/null
+++ b/core/src/int_storage.h
@@ -0,0 +1,40 @@
+#ifndef _G3_INT_STORAGE_H
+#define _G3_INT_STORAGE_H
+
+#include <pybindings.h>
+#include <serialization.h>
+#include <vector>
+#include <string>
+#include <map>
+
+int bit_count(const std::vector<int64_t> &d);
+int bit_count(const std::map<std::string, int64_t> &d);
+
+template <class A, typename FROM_TYPE>
+void load_as(A &ar, std::vector<int64_t> &dest) {
+	std::vector<FROM_TYPE> temp;
+	ar & cereal::make_nvp("vector", temp);
+	dest.resize(temp.size());
+	std::copy(temp.begin(), temp.end(), dest.begin());
+}
+
+template <class A, typename FROM_TYPE>
+void load_as(A &ar, std::map<std::string, int64_t> &dest) {
+	std::map<std::string, FROM_TYPE> temp;
+	ar & cereal::make_nvp("map", temp);
+	dest.insert(temp.begin(), temp.end());
+}
+
+template <class A, typename TO_TYPE>
+void save_as(A &ar, const std::vector<int64_t> &src) {
+	std::vector<TO_TYPE> temp(src.begin(), src.end());
+	ar & cereal::make_nvp("vector", temp);
+}
+
+template <class A, typename TO_TYPE>
+void save_as(A &ar, const std::map<std::string, int64_t> &src) {
+	std::map<std::string, TO_TYPE> temp(src.begin(), src.end());
+	ar & cereal::make_nvp("map", temp);
+}
+
+#endif


Test patch:
diff --git a/core/tests/vecint.py b/core/tests/vecint.py
index 51df851f..4efb4878 100644
--- a/core/tests/vecint.py
+++ b/core/tests/vecint.py
@@ -50,6 +50,42 @@ def test_serialize(self):
                              "Failed to save/load value %i" % val)
         del r
 
+    def test_serialize_map(self):
+        """Confirm full ranges can be saved and loaded."""
+
+        w = core.G3Writer(test_filename)
+        for isize, val in bit_sizes:
+            f = core.G3Frame()
+            f['m'] = core.G3MapInt({str(i): val for i in range(10)})
+            w(f)
+        del w
+
+        r = core.G3Reader(test_filename)
+        for isize, val in bit_sizes:
+            f = r(None)[0]
+            v_in = list(f['m'].values())
+            self.assertTrue(all([_v == val for _v in v_in]),
+                             "Failed to save/load value %i" % val)
+        del r
+
+    def test_serialize_map_vector(self):
+        """Confirm full ranges can be saved and loaded."""
+
+        w = core.G3Writer(test_filename)
+        for isize, val in bit_sizes:
+            f = core.G3Frame()
+            f['m'] = core.G3MapVectorInt({'a': [val] * 10})
+            w(f)
+        del w
+
+        r = core.G3Reader(test_filename)
+        for isize, val in bit_sizes:
+            f = r(None)[0]
+            v_in = list(f['m']['a'])
+            self.assertTrue(all([_v == val for _v in v_in]),
+                             "Failed to save/load value %i" % val)
+        del r
+
     def test_compression(self):
         """Confirm that minimum necessary int size is used for serialization."""
         count = 10000
@@ -66,5 +102,37 @@ def test_compression(self):
                             "too far from %.2f bytes/item" %
                             (val, on_disk / count, isize / 8))
 
+    def test_compression_map(self):
+        """Confirm that minimum necessary int size is used for serialization."""
+        count = 10000
+        overhead = 12
+        for isize, val in bit_sizes:
+            w = core.G3Writer(test_filename)
+            f = core.G3Frame()
+            f['v'] = core.G3MapInt({str(i): val for i in range(count)})
+            w(f)
+            del w
+            on_disk = os.path.getsize(test_filename)
+            self.assertTrue(abs(on_disk - count * isize / 8) <= (overhead * count),
+                            "Storage for val %i took %.2f bytes/item, "
+                            "too far from %.2f bytes/item" %
+                            (val, on_disk / count - overhead, isize / 8))
+
+    def test_compression_map_vector(self):
+        """Confirm that minimum necessary int size is used for serialization."""
+        count = 10000
+        overhead = 200
+        for isize, val in bit_sizes:
+            w = core.G3Writer(test_filename)
+            f = core.G3Frame()
+            f['v'] = core.G3MapVectorInt({'a': [val] * count})
+            w(f)
+            del w
+            on_disk = os.path.getsize(test_filename)
+            self.assertTrue(abs(on_disk - count * isize / 8) <= overhead,
+                            "Storage for val %i took %.2f bytes/item, "
+                            "too far from %.2f bytes/item" %
+                            (val, on_disk / count, isize / 8))
+
 if __name__ == '__main__':
     unittest.main()