Merge pull request #668 from MesserLab/fix_666

shift the migrant flag from metadata to the flags column

Author: bhaller

VERSIONS

@@ -212,6 +212,8 @@ multitrait branch:
 	    however, we do not rely on the derived state being anything when reading in a .trees file
 		in-memory, the binary derived state info remains in the derived state column and the mutation table metadata is kept empty, as before
 		the schema for the mutation table metadata now encodes an array (of length determined by the metadata itself) of int64_t SLiM mutation IDs
+	fix #666, remove `flags` from individual metadata (shifting the migrant flag, now named SLIM_TSK_INDIVIDUAL_MIGRATED, over to the flags column of the individual table)
+		note this entails changes to the individual table flags column, the individual table metadata and schema, and SLiM's provenance entries (to add a description of the flag)
 
 
 version 5.2 (Eidos version 4.2):

core/slim_globals.cpp

@@ -2284,7 +2284,6 @@ const std::string gSLiM_tsk_individual_metadata_schema_FORMAT_SOURCE = R"V0G0N({
     "examples": [
         {
             "age": -1,
-            "flags": 0,
             "pedigree_id": 123,
             "pedigree_p1": 12,
             "pedigree_p2": 23,
@@ -2310,12 +2309,6 @@ const std::string gSLiM_tsk_individual_metadata_schema_FORMAT_SOURCE = R"V0G0N({
             "tagL4": false
         }
     ],
-    "flags": {
-        "SLIM_INDIVIDUAL_METADATA_MIGRATED": {
-            "description": "Whether this individual was a migrant, either in the tick when the tree sequence was written out (if the individual was alive then), or in the tick of the last time they were Remembered (if not).",
-            "value": 1
-        }
-    },
     "properties": {
         "pedigree_id": {
             "index": 1,
@@ -2335,104 +2328,98 @@ const std::string gSLiM_tsk_individual_metadata_schema_FORMAT_SOURCE = R"V0G0N({
             "binaryFormat": "q",
             "description": "The 'pedigree ID' of this individual's second parent in SLiM."
         },
+        "tag": {
+			"index": 4,
+			"type": "integer",
+			"binaryFormat": "q",
+			"description": "The `tag` property of this individual; INT64_MIN if unset."
+        },
+        "tagF": {
+			"index": 5,
+			"type": "number",
+			"binaryFormat": "d",
+			"description": "The `tagF` property of this individual; -DBL_MAX if unset."
+        },
         "age": {
-            "index": 4,
+            "index": 6,
             "type": "integer",
             "binaryFormat": "i",
             "description": "The age of this individual, either when the tree sequence was written out (if the individual was alive then), or the last time they were Remembered (if not)."
         },
         "subpopulation": {
-            "index": 5,
+            "index": 7,
             "type": "integer",
             "binaryFormat": "i",
             "description": "The ID of the subpopulation the individual was part of, either when the tree sequence was written out (if the individual was alive then), or the last time they were Remembered (if not)."
         },
         "sex": {
-            "index": 6,
+            "index": 8,
             "type": "integer",
-            "binaryFormat": "i",
+            "binaryFormat": "h",
             "description": "The sex of the individual (0 for female, 1 for male, -1 for hermaphrodite)."
         },
-        "flags": {
-            "index": 7,
-            "type": "integer",
-            "binaryFormat": "I",
-            "description": "Other information about the individual: see 'flags'."
-        },
-        "tag": {
-			"index": 8,
-			"type": "integer",
-			"binaryFormat": "q",
-			"description": "The `tag` property of this individual; INT64_MIN if unset."
-        },
-        "tagF": {
-			"index": 9,
-			"type": "number",
-			"binaryFormat": "d",
-			"description": "The `tagF` property of this individual; -DBL_MAX if unset."
-        },
         "tagL0_set": {
-			"index": 10,
+			"index": 9,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "A flag indicating whether the `tagL0` property is set; if false, accessing `tagL0` is invalid."
         },
         "tagL0": {
-			"index": 11,
+			"index": 10,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "The `tagL0` property of this individual; only valid if `tagL0_set` is true."
         },
         "tagL1_set": {
-			"index": 12,
+			"index": 11,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "A flag indicating whether the `tagL1` property is set; if false, accessing `tagL1` is invalid."
         },
         "tagL1": {
-			"index": 13,
+			"index": 12,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "The `tagL1` property of this individual; only valid if `tagL1_set` is true."
         },
         "tagL2_set": {
-			"index": 14,
+			"index": 13,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "A flag indicating whether the `tagL2` property is set; if false, accessing `tagL2` is invalid."
         },
         "tagL2": {
-			"index": 15,
+			"index": 14,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "The `tagL2` property of this individual; only valid if `tagL2_set` is true."
         },
         "tagL3_set": {
-			"index": 16,
+			"index": 15,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "A flag indicating whether the `tagL3` property is set; if false, accessing `tagL3` is invalid."
         },
         "tagL3": {
-			"index": 17,
+			"index": 16,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "The `tagL3` property of this individual; only valid if `tagL3_set` is true."
         },
         "tagL4_set": {
-			"index": 18,
+			"index": 17,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "A flag indicating whether the `tagL4` property is set; if false, accessing `tagL4` is invalid."
         },
         "tagL4": {
-			"index": 19,
+			"index": 18,
 			"type": "boolean",
 			"binaryFormat": "?",
 			"description": "The `tagL4` property of this individual; only valid if `tagL4_set` is true."
         },
         "per_trait": {
-            "index": 20,
+            "index": 19,
             "type": "array",
             "length": "%d",
             "items": {
@@ -2464,11 +2451,11 @@ const std::string gSLiM_tsk_individual_metadata_schema_FORMAT_SOURCE = R"V0G0N({
         "pedigree_id",
         "pedigree_p1",
         "pedigree_p2",
+        "tag",
+        "tagF",
         "age",
         "subpopulation",
         "sex",
-        "tag",
-        "tagF",
         "tagL0_set",
         "tagL0",
         "tagL1_set",
@@ -2479,7 +2466,6 @@ const std::string gSLiM_tsk_individual_metadata_schema_FORMAT_SOURCE = R"V0G0N({
         "tagL3",
         "tagL4_set",
         "tagL4",
-        "flags",
         "per_trait"
     ]
 })V0G0N";

core/slim_globals.h

@@ -717,6 +717,7 @@ extern EidosPalette *gEidos_Palette_MutationLogisticEffect;
 #define SLIM_TSK_INDIVIDUAL_ALIVE       ((tsk_flags_t)(1 << 16))
 #define SLIM_TSK_INDIVIDUAL_REMEMBERED  ((tsk_flags_t)(1 << 17))
 #define SLIM_TSK_INDIVIDUAL_RETAINED    ((tsk_flags_t)(1 << 18))
+#define SLIM_TSK_INDIVIDUAL_MIGRATED	((tsk_flags_t)(1 << 19))
 
 extern const std::string *gSLiM_tsk_metadata_JSON_schema;
 extern const std::string *gSLiM_tsk_metadata_binary_schema_FORMAT;

core/species.cpp

@@ -9018,8 +9018,9 @@ void Species::AddIndividualsToTable(Individual * const *p_individual, size_t p_n
 		if (ind_pos == p_individuals_hash->end()) {
 			// This individual is not already in the tables.
 			tsk_id_t tsk_individual = tsk_individual_table_add_row(&p_tables->individuals,
-					p_flags, location.data(), (uint32_t)location.size(), 
-                    NULL, 0, // individual parents
+					p_flags | (ind->migrant_ ? SLIM_TSK_INDIVIDUAL_MIGRATED : 0),	// add migrant flag if needed
+					location.data(), (uint32_t)location.size(), 
+                    NULL, 0,	// individual parents; fixed later by AddParentsColumnForOutput()
 					(char *)&metadata_rec, (uint32_t)total_metadata_size);
 			if (tsk_individual < 0) handle_error("tsk_individual_table_add_row", tsk_individual);
 
@@ -9054,24 +9055,37 @@ void Species::AddIndividualsToTable(Individual * const *p_individual, size_t p_n
 			// It could have been previously inserted but not with the SLIM_TSK_INDIVIDUAL_REMEMBERED
 			// flag: if so, it now needs adding to the list of remembered nodes
 			tsk_id_t tsk_node_id_base = ind->TskitNodeIdBase();
+			tsk_flags_t &individual_row_flags = p_tables->individuals.flags[tsk_individual];
 
-			if (((p_tables->individuals.flags[tsk_individual] & SLIM_TSK_INDIVIDUAL_REMEMBERED) == 0)
+			if (((individual_row_flags & SLIM_TSK_INDIVIDUAL_REMEMBERED) == 0)
 				&& (p_flags & SLIM_TSK_INDIVIDUAL_REMEMBERED))
 			{
 				remembered_nodes_.emplace_back(tsk_node_id_base);
 				remembered_nodes_.emplace_back(tsk_node_id_base + 1);
 			}
-
+			
 			memcpy(p_tables->individuals.location
 				   + p_tables->individuals.location_offset[tsk_individual],
 				   location.data(), location.size() * sizeof(double));
 			memcpy(p_tables->individuals.metadata
 					+ p_tables->individuals.metadata_offset[tsk_individual],
 					&metadata_rec, total_metadata_size);
-			p_tables->individuals.flags[tsk_individual] |= p_flags;
+			
+			// BCH 8/26/2026: Note that we OR flags in here, and never clear flags, because of the way this
+			// method is called; individuals that get remembered/retained get set with only that flag, and
+			// then we call this method again later for alive individuals and want to just add the alive flag.
+			individual_row_flags |= p_flags;
+			
+			// BCH 8/26/2026: We have to fix the `migrant` flag, which might have changed value since this
+			// individual was last recorded.  This used to be done by the memcpy() of the metadata above,
+			// but now the `migrant` flag is kept in the flags column, not the metadata.
+			if (ind->migrant_)
+				individual_row_flags |= SLIM_TSK_INDIVIDUAL_MIGRATED;
+			else
+				individual_row_flags &= ~SLIM_TSK_INDIVIDUAL_MIGRATED;
 
 			// Check node table
-			assert(ind->TskitNodeIdBase() + 1 < (tsk_id_t) p_tables->nodes.num_rows);	// base and base+1 must both be in range
+			assert(tsk_node_id_base + 1 < (tsk_id_t) p_tables->nodes.num_rows);	// base and base+1 must both be in range
 
 			// BCH 4/29/2019: These asserts are, we think, not technically necessary – the code
 			// would work even if they were violated.  But they're a nice invariant to guarantee,
@@ -9885,6 +9899,7 @@ void Species::WriteProvenanceTable(tsk_table_collection_t *p_tables, bool p_use_
 
 		// BCH 3/10/2024: Moving from schema version 1.0.0 to 1.1.0, which I believe shipped in tskit 0.5.9.
 		// This adds the optional `resources` key.  See https://github.com/MesserLab/SLiM/issues/478.
+		// See https://tskit.dev/tskit/docs/stable/provenance.html#full-schema for the current provenance schema.
 		j["schema_version"] = "1.1.0";
 
 		struct utsname name;
@@ -10042,6 +10057,8 @@ void Species::WriteProvenanceTable(tsk_table_collection_t *p_tables, bool p_use_
 		j["metadata"]["individuals"]["flags"]["17"]["description"] = "the individual was requested by the user to be permanently remembered";
 		j["metadata"]["individuals"]["flags"]["18"]["name"] = "SLIM_TSK_INDIVIDUAL_RETAINED";
 		j["metadata"]["individuals"]["flags"]["18"]["description"] = "the individual was requested by the user to be retained only if its nodes continue to exist in the tree sequence";
+		j["metadata"]["individuals"]["flags"]["19"]["name"] = "SLIM_TSK_INDIVIDUAL_MIGRATED";
+		j["metadata"]["individuals"]["flags"]["19"]["description"] = "the individual is a recent migrant between subpopulations";
 
 		// We save this information out only for runs at the command line.  This data might not be available on
 		// all platforms; when it is unavailable, the key will be omitted.  We always have elapsed wall time.
@@ -11064,34 +11081,33 @@ void Species::MetadataForIndividual(Individual *p_individual, IndividualMetadata
 	// We check the struct size here to detect changes that would need to be responded to here; but it is
 	// very important to note that the caller guarantees that the actual size of p_metadata is large enough
 	// to accommodate all of the per-trait metadata, which is variable-length!
-	static_assert(sizeof(IndividualMetadataRec) == 66, "IndividualMetadataRec has changed size; this code probably needs to be updated");
+	static_assert(sizeof(IndividualMetadataRec) == 60, "IndividualMetadataRec has changed size; this code probably needs to be updated");
 
 #if DEBUG
 	if (!p_individual || !p_metadata)
 		EIDOS_TERMINATION << "ERROR (Species::MetadataForIndividual): (internal error) bad parameters to MetadataForIndividual()." << EidosTerminate();
 #endif
 
+	// Note: tag values were added to the individual metadata in SLiM 6.0; we write false for
+	// unset values to avoid accessing unwritten memory, which would be flagged by UBSan
+	
 	p_metadata->pedigree_id_ = p_individual->PedigreeID();
 	p_metadata->pedigree_p1_ = p_individual->Parent1PedigreeID();
 	p_metadata->pedigree_p2_ = p_individual->Parent2PedigreeID();
+	p_metadata->tag_ = p_individual->tag_value_;
+	p_metadata->tagF_ = p_individual->tagF_value_;
+	
 	p_metadata->age_ = p_individual->age_;
 	p_metadata->subpopulation_id_ = p_individual->subpopulation_->subpopulation_id_;
-	p_metadata->sex_ = (int32_t)p_individual->sex_;		// IndividualSex, but int32_t in the record
 
-	p_metadata->flags_ = 0;
-	if (p_individual->migrant_)
-		p_metadata->flags_ |= SLIM_INDIVIDUAL_METADATA_MIGRATED;
-	
-	// tag values added to the metadata in SLiM 6.0; we write false for unset values
-	// to avoid accessing unwritten memory, which would be flagged by UBSan
-	p_metadata->tag_ = p_individual->tag_value_;
-	p_metadata->tagF_ = p_individual->tagF_value_;
+	p_metadata->sex_ = (int16_t)p_individual->sex_;		// IndividualSex, but int16_t in the record
 	p_metadata->tagL0_set_ = p_individual->tagL0_set_;
 	p_metadata->tagL0_ = p_individual->tagL0_set_ ? p_individual->tagL0_value_ : false;
 	p_metadata->tagL1_set_ = p_individual->tagL1_set_;
 	p_metadata->tagL1_ = p_individual->tagL1_set_ ? p_individual->tagL1_value_ : false;
 	p_metadata->tagL2_set_ = p_individual->tagL2_set_;
 	p_metadata->tagL2_ = p_individual->tagL2_set_ ? p_individual->tagL2_value_ : false;
+	
 	p_metadata->tagL3_set_ = p_individual->tagL3_set_;
 	p_metadata->tagL3_ = p_individual->tagL3_set_ ? p_individual->tagL3_value_ : false;
 	p_metadata->tagL4_set_ = p_individual->tagL4_set_;
@@ -11911,6 +11927,7 @@ void Species::__RemapSubpopulationIDs(SUBPOP_REMAP_HASH &p_subpop_map, TreeSeqIn
 typedef struct ts_subpop_info {
 	slim_popsize_t countMH_ = 0, countF_ = 0;
 	std::vector<tsk_id_t> nodes_;
+	std::vector<bool> migrant_flags_;						// true for each individual if SLIM_TSK_INDIVIDUAL_MIGRATED is set
 	std::vector<const double*> spatial_positions_;			// points into the locations column of the individual table
 	std::vector<const IndividualMetadataRec*> metadata_;	// points into the metadata column of the individual table
 } ts_subpop_info;
@@ -11993,6 +12010,10 @@ void Species::__TabulateSubpopulationsFromTreeSequence(std::unordered_map<slim_o
 
 		ts_subpop_info &subpop_info = subpop_info_iter->second;
 
+		// remember the SLIM_TSK_INDIVIDUAL_MIGRATED flag from the flags column of the individual table
+		// this used to be in metadata, but now that it is in the flags column we need to keep it separately
+		subpop_info.migrant_flags_.push_back(!!(individual.flags & SLIM_TSK_INDIVIDUAL_MIGRATED));
+		
 		// remember our metadata pointer, we will fetch information from it later; below we check metadata for
 		// correctness, and even edit it, but we do not copy its values, we just keep the metadata pointer
 		subpop_info.metadata_.push_back(metadata);
@@ -12260,8 +12281,7 @@ void Species::__CreateSubpopulationsFromTabulation(std::unordered_map<slim_objec
 
 				individual->SetParentPedigreeID(ind_metadata->pedigree_p1_, ind_metadata->pedigree_p2_);
 
-				uint32_t flags = ind_metadata->flags_;
-				if (flags & SLIM_INDIVIDUAL_METADATA_MIGRATED)
+				if (subpop_info.migrant_flags_[tabulation_index])
 					individual->migrant_ = true;
 
 				individual->age_ = ind_metadata->age_;
@@ -12480,8 +12500,7 @@ void Species::__CreateSubpopulationsFromTabulation_SECONDARY(std::unordered_map<
 					(individual->Parent2PedigreeID() != ind_metadata->pedigree_p2_))
 					EIDOS_TERMINATION << "ERROR (Species::__CreateSubpopulationsFromTabulation_SECONDARY): parent pedigree id mismatch between chromosomes read." << EidosTerminate();
 
-				uint32_t flags = ind_metadata->flags_;
-				if ((flags & SLIM_INDIVIDUAL_METADATA_MIGRATED) && !individual->migrant_)
+				if (subpop_info.migrant_flags_[tabulation_index] != individual->migrant_)
 					EIDOS_TERMINATION << "ERROR (Species::__CreateSubpopulationsFromTabulation_SECONDARY): individual migrant flag mismatch between chromosomes read." << EidosTerminate();
 
 				if (individual->age_ != ind_metadata->age_)

core/species.h

@@ -145,37 +145,37 @@ typedef struct __attribute__((__packed__)) {
 	slim_pedigreeid_t pedigree_id_;			// 8 bytes (int64_t): the SLiM pedigree ID for this individual, assigned by pedigree rec
 	slim_pedigreeid_t pedigree_p1_;			// 8 bytes (int64_t): the SLiM pedigree ID for this individual's parent 1
 	slim_pedigreeid_t pedigree_p2_;			// 8 bytes (int64_t): the SLiM pedigree ID for this individual's parent 2
-	slim_age_t age_;                        // 4 bytes (int32_t): the age of the individual (-1 for WF models)
-	slim_objectid_t subpopulation_id_;      // 4 bytes (int32_t): the subpopulation the individual belongs to
-	int32_t sex_;							// 4 bytes (int32_t): the sex of the individual, as defined by the IndividualSex enum
-	uint32_t flags_;						// 4 bytes (uint32_t): assorted flags, see below
 	int64_t tag_;							// 8 bytes (int64_t): the `tag` property value
 	double tagF_;							// 8 bytes (double): the `tagF` property value
+	
+	slim_age_t age_;                        // 4 bytes (int32_t): the age of the individual (-1 for WF models)
+	slim_objectid_t subpopulation_id_;      // 4 bytes (int32_t): the subpopulation the individual belongs to
+	
+	int16_t sex_;							// 2 bytes (int16_t): the sex of the individual, as defined by the IndividualSex enum
 	uint8_t tagL0_set_;						// 1 byte (uint8_t): a flag indicating whether `tagL0` is set
 	uint8_t tagL0_;							// 1 byte (uint8_t): the `tagL0` property value
 	uint8_t tagL1_set_;						// 1 byte (uint8_t): a flag indicating whether `tagL1` is set
 	uint8_t tagL1_;							// 1 byte (uint8_t): the `tagL1` property value
 	uint8_t tagL2_set_;						// 1 byte (uint8_t): a flag indicating whether `tagL2` is set
 	uint8_t tagL2_;							// 1 byte (uint8_t): the `tagL2` property value
+	
 	uint8_t tagL3_set_;						// 1 byte (uint8_t): a flag indicating whether `tagL3` is set
 	uint8_t tagL3_;							// 1 byte (uint8_t): the `tagL3` property value
 	uint8_t tagL4_set_;						// 1 byte (uint8_t): a flag indicating whether `tagL4` is set
 	uint8_t tagL4_;							// 1 byte (uint8_t): the `tagL4` property value
 
-	// NOTE: after this point the packed struct is only aligned to a two-byte boundary!
+	// NOTE: after this point the packed struct is only aligned to a four-byte boundary!
 
 	_IndividualPerTraitMetadata per_trait_[];	// 16 bytes per entry: 0 or more per-trait entries (count determined by the schema!)
 } IndividualMetadataRec;
 
-#define SLIM_INDIVIDUAL_METADATA_MIGRATED	0x01	// set if the individual has migrated in this cycle
-
 // We double-check the size of these records to make sure we understand what they contain and how they're packed
 // BCH 2/11/2026: Note that all of these metadata structs are now actually variable-length; this is just a base.
 static_assert(sizeof(_MutationPerTraitMetadata) == 12, "_MutationPerTraitMetadata is not 12 bytes!");
 static_assert(sizeof(MutationTableMetadataRec) == 36, "MutationTableMetadataRec is not 36 bytes!");
 static_assert(sizeof(HaplosomeMetadataRec) == 9, "HaplosomeMetadataRec is not 9 bytes!");
 static_assert(sizeof(_IndividualPerTraitMetadata) == 16, "_IndividualPerTraitMetadata is not 16 bytes!");
-static_assert(sizeof(IndividualMetadataRec) == 66, "IndividualMetadataRec is not 66 bytes!");
+static_assert(sizeof(IndividualMetadataRec) == 60, "IndividualMetadataRec is not 60 bytes!");
 
 // We check endianness on the platform we're building on; we assume little-endianness in our read/write code, I think.
 #if defined(__BYTE_ORDER__)

treerec/tests/.python-version

@@ -0,0 +1 @@
+3.13