Skip to content

Commit

Permalink
fix #418, crash involving null genomes in nonWF models
Browse files Browse the repository at this point in the history
  • Loading branch information
@bhaller
bhaller committed Dec 11, 2023
1 parent c207f65 commit 889bbb3
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 32 deletions.
1 change: 1 addition & 0 deletions VERSIONS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Note that not every commit will be logged here; that is what the Github commit h


development head (in the master branch):
fix for #418, a crash involving null genomes in nonWF (has_null_genomes_ was not set correctly by addCloned() or takeMigrants() when putting a null genome into a subpop that previously had none)


version 4.1 (Eidos version 3.1):
Expand Down
63 changes: 32 additions & 31 deletions core/population.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5985,33 +5985,33 @@ slim_refcount_t Population::TallyMutationRunReferencesForPopulation(void)
slim_popsize_t subpop_genome_count = subpop->CurrentGenomeCount();
std::vector<Genome *> &subpop_genomes = subpop->CurrentGenomes();

if ((species_.ModeledChromosomeType() == GenomeType::kAutosome) && !subpop->has_null_genomes_)
if (subpop->CouldContainNullGenomes())
{
// optimized case when null genomes do not exist in this subpop
for (slim_popsize_t i = 0; i < subpop_genome_count; i++)
{
Genome &genome = *subpop_genomes[i];

for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();
if (!genome.IsNull())
{
for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();

total_genome_count++;
}
}

total_genome_count += subpop_genome_count;
}
else
{
// optimized case when null genomes do not exist in this subpop
for (slim_popsize_t i = 0; i < subpop_genome_count; i++)
{
Genome &genome = *subpop_genomes[i];

if (!genome.IsNull())
{
for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();

total_genome_count++;
}
for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();
}

total_genome_count += subpop_genome_count;
}
}
}
Expand Down Expand Up @@ -6125,33 +6125,33 @@ slim_refcount_t Population::TallyMutationRunReferencesForSubpops(std::vector<Sub
slim_popsize_t subpop_genome_count = subpop->CurrentGenomeCount();
std::vector<Genome *> &subpop_genomes = subpop->CurrentGenomes();

if ((species_.ModeledChromosomeType() == GenomeType::kAutosome) && !subpop->has_null_genomes_)
if (subpop->CouldContainNullGenomes())
{
// optimized case when null genomes do not exist in this subpop
for (slim_popsize_t i = 0; i < subpop_genome_count; i++)
{
Genome &genome = *subpop_genomes[i];

for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();
if (!genome.IsNull())
{
for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();

total_genome_count++;
}
}

total_genome_count += subpop_genome_count;
}
else
{
// optimized case when null genomes do not exist in this subpop
for (slim_popsize_t i = 0; i < subpop_genome_count; i++)
{
Genome &genome = *subpop_genomes[i];

if (!genome.IsNull())
{
for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();

total_genome_count++;
}
for (int run_index = first_mutrun_index; run_index <= last_mutrun_index; ++run_index)
genome.mutruns_[run_index]->increment_use_count();
}

total_genome_count += subpop_genome_count;
}
}
}
Expand Down Expand Up @@ -6267,11 +6267,7 @@ slim_refcount_t Population::_CountNonNullGenomes(void)

slim_popsize_t subpop_genome_count = subpop->CurrentGenomeCount();

if ((species_.ModeledChromosomeType() == GenomeType::kAutosome) && !subpop->has_null_genomes_)
{
total_genome_count += subpop_genome_count;
}
else
if (subpop->CouldContainNullGenomes())
{
std::vector<Genome *> &subpop_genomes = subpop->CurrentGenomes();

Expand All @@ -6283,6 +6279,11 @@ slim_refcount_t Population::_CountNonNullGenomes(void)
total_genome_count++;
}
}
else
{
// optimized case when null genomes do not exist in this subpop
total_genome_count += subpop_genome_count;
}
}

return total_genome_count;
Expand Down
31 changes: 31 additions & 0 deletions core/subpopulation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,24 @@ void Subpopulation::SetName(const std::string &p_name)
name_ = p_name;
}

slim_refcount_t Subpopulation::NullGenomeCount(void)
{
slim_refcount_t null_genome_count = 0;

slim_popsize_t subpop_genome_count = CurrentGenomeCount();
std::vector<Genome *> &subpop_genomes = CurrentGenomes();

for (slim_popsize_t i = 0; i < subpop_genome_count; i++)
{
Genome &genome = *subpop_genomes[i];

if (genome.IsNull())
null_genome_count++;
}

return null_genome_count;
}

#if (defined(_OPENMP) && SLIM_USE_NONNEUTRAL_CACHES)
void Subpopulation::FixNonNeutralCaches_OMP(void)
{
Expand Down Expand Up @@ -4481,6 +4499,9 @@ EidosValue_SP Subpopulation::ExecuteMethod_addCloned(EidosGlobalStringID p_metho
bool genome1_null = parent->genome1_->IsNull(), genome2_null = parent->genome2_->IsNull();
IndividualSex child_sex = parent_sex;

if (genome1_null || genome2_null)
has_null_genomes_ = true;

// Generate the number of children requested
Chromosome &chromosome = species_.TheChromosome();
int32_t mutrun_count = chromosome.mutrun_count_;
Expand Down Expand Up @@ -5711,6 +5732,11 @@ EidosValue_SP Subpopulation::ExecuteMethod_takeMigrants(EidosGlobalStringID p_me
parent_individuals_[parent_first_male_index_] = migrant;
parent_genomes_[(size_t)parent_first_male_index_ * 2] = migrant->genome1_;
parent_genomes_[(size_t)parent_first_male_index_ * 2 + 1] = migrant->genome2_;

// the has_null_genomes_ needs to reflect the presence of null genomes
if (migrant->genome1_->IsNull() || migrant->genome2_->IsNull())
has_null_genomes_ = true;

migrant->subpopulation_ = this;
migrant->index_ = parent_first_male_index_;

Expand All @@ -5723,6 +5749,11 @@ EidosValue_SP Subpopulation::ExecuteMethod_takeMigrants(EidosGlobalStringID p_me
parent_individuals_.emplace_back(migrant);
parent_genomes_.emplace_back(migrant->genome1_);
parent_genomes_.emplace_back(migrant->genome2_);

// the has_null_genomes_ needs to reflect the presence of null genomes
if (migrant->genome1_->IsNull() || migrant->genome2_->IsNull())
has_null_genomes_ = true;

migrant->subpopulation_ = this;
migrant->index_ = parent_subpop_size_;

Expand Down
15 changes: 14 additions & 1 deletion core/subpopulation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ class Subpopulation : public EidosDictionaryUnretained
EidosObjectPool &individual_pool_; // NOT OWNED: a pool out of which individuals are allocated, for within-species locality of memory usage across individuals
std::vector<Genome *> &genome_junkyard_nonnull; // NOT OWNED: non-null genomes get put here when we're done with them, so we can reuse them without dealloc/realloc of their mutrun buffers
std::vector<Genome *> &genome_junkyard_null; // NOT OWNED: null genomes get put here when we're done with them, so we can reuse them without dealloc/realloc of their mutrun buffers
bool has_null_genomes_ = false; // false until addRecombinant() etc. generates a null genome and sets it to true; NOT set by null genomes for sex chromosome sims; used for optimizations
bool has_null_genomes_ = false; // false until a null genome is added; NOT set by null genomes for sex chromosome sims; use CouldContainNullGenomes() to check this flag

std::vector<Genome *> parent_genomes_; // OWNED: all genomes in the parental generation; each individual gets two genomes, males are XY (not YX)
EidosValue_SP cached_parent_genomes_value_; // cached for the genomes property; reset() if changed
Expand Down Expand Up @@ -251,6 +251,19 @@ class Subpopulation : public EidosDictionaryUnretained

void SetName(const std::string &p_name); // change the name property of the subpopulation, handling the uniqueness logic

slim_refcount_t NullGenomeCount(void);
inline bool CouldContainNullGenomes(void) {
// sex-chromosome simulations can always contain null genomes
if (species_.ModeledChromosomeType() != GenomeType::kAutosome)
return true;
#if DEBUG
// in DEBUG, check that has_null_genomes_ is not false when null genomes in fact exist (we allow the opposite case)
if (!has_null_genomes_ && (NullGenomeCount() > 0))
EIDOS_TERMINATION << "ERROR (Subpopulation::CouldContainNullGenomes): (internal error) has_null_genomes_ is not correct." << EidosTerminate();
#endif
return (has_null_genomes_);
}

slim_popsize_t DrawParentUsingFitness(gsl_rng *rng) const; // WF only: draw an individual from the subpopulation based upon fitness
slim_popsize_t DrawFemaleParentUsingFitness(gsl_rng *rng) const; // WF only: draw a female from the subpopulation based upon fitness; SEX ONLY
slim_popsize_t DrawMaleParentUsingFitness(gsl_rng *rng) const; // WF only: draw a male from the subpopulation based upon fitness; SEX ONLY
Expand Down

0 comments on commit 889bbb3

Please sign in to comment.