Fix #92 - Multiple occurances of same chromosome instance in generation

giacomelli · Sep 9, 2022 · 381bc8e · 381bc8e
1 parent 5a2c2f5
commit 381bc8e
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 51 deletions.
diff --git a/src/GeneticSharp.Domain.UnitTests/Chromosomes/ChromosomeBaseTest.cs b/src/GeneticSharp.Domain.UnitTests/Chromosomes/ChromosomeBaseTest.cs
@@ -125,22 +125,6 @@ public void ReplaceGenes_NullGenes_Exception()
             });
         }
 
-        [Test]
-        public void ReplaceGenes_GenesExceedChromosomeLength_Exception()
-        {
-            var target = Substitute.For<ChromosomeBase>(3);
-
-            Assert.Catch<ArgumentException>(() =>
-            {
-                target.ReplaceGenes(0, new Gene[] { new Gene(1), new Gene(2), new Gene(3), new Gene(4) });
-            }, "The number of genes to be replaced is greater than available space, there is 3 genes between the index 0 and the end of chromosome, but there is 4 genes to be replaced.");
-
-            Assert.Catch<ArgumentException>(() =>
-            {
-                target.ReplaceGenes(1, new Gene[] { new Gene(1), new Gene(2), new Gene(3) });
-            }, "The number of genes to be replaced is greater than available space, there is 2 genes between the index 1 and the end of chromosome, but there is 3 genes to be replaced.");
-        }
-
         [Test]
         public void ReplaceGenes_ValidIndex_Replaced()
         {

diff --git a/src/GeneticSharp.Domain.UnitTests/GeneticAlgorithmTest.cs b/src/GeneticSharp.Domain.UnitTests/GeneticAlgorithmTest.cs
@@ -4,6 +4,7 @@
 using System.Threading.Tasks;
 using NUnit.Framework;
 using NSubstitute;
+using System.Security.Cryptography;
 
 namespace GeneticSharp.Domain.UnitTests
 {
@@ -536,7 +537,15 @@ public void Start_UsingAllConfigurationCombinationsAvailable_AllRun()
                             target.Termination = new GenerationNumberTermination(25);
                             target.CrossoverProbability = reinsertion.CanExpand ? 0.75f : 1f;
 
-                            target.Start();
+                            try
+                            {
+                                target.Start();
+                            }
+                            catch(Exception ex)
+                            {
+                                throw new Exception($"GA start failed using selection:{s}, crossover:{c}, mutation:{m} and reinsertion:{r}. Error: {ex.Message}", ex);
+                            }
+
                             Assert.AreEqual(25, target.Population.Generations.Count);
                         }
                     }

diff --git a/src/GeneticSharp.Domain.UnitTests/Selections/RankSelectionTest.cs b/src/GeneticSharp.Domain.UnitTests/Selections/RankSelectionTest.cs
@@ -4,6 +4,7 @@
 using NUnit.Framework;
 using NSubstitute;
 using GeneticSharp.Extensions;
+using NSubstitute.Routing.Handlers;
 
 namespace GeneticSharp.Domain.UnitTests.Selections
 {
@@ -17,7 +18,7 @@ public void Cleanup()
             RandomizationProvider.Current = new BasicRandomization();
         }
 
-        [Test()]
+       [Test]
         public void SelectChromosomes_InvalidNumber_Exception()
         {
             var target = new RankSelection();
@@ -38,7 +39,7 @@ public void SelectChromosomes_InvalidNumber_Exception()
             }, "The number of selected chromosomes should be at least 2.");
         }
 
-        [Test()]
+       [Test]
         public void SelectChromosomes_NullGeneration_Exception()
         {
             var target = new RankSelection();
@@ -51,7 +52,7 @@ public void SelectChromosomes_NullGeneration_Exception()
             Assert.AreEqual("generation", actual.ParamName);
         }
 
-        [Test()]
+       [Test]
         public void SelectChromosomes_Generation_ChromosomesSelected()
         {
             var target = new RankSelection();
@@ -157,7 +158,7 @@ public void SelectChromosomes_NullFitness_Exception()
             Assert.AreEqual("RankSelection: There are chromosomes with null fitness.", actual.Message);
         }
 
-        [Test()]
+       [Test]
         public void SelectChromosomes_Generation_ChromosomesZeroFitness()
         {
             var target = new RankSelection();
@@ -179,6 +180,6 @@ public void SelectChromosomes_Generation_ChromosomesZeroFitness()
 
             var actual = target.SelectChromosomes(2, generation);
             Assert.AreEqual(2, actual.Count);
-        }
+        }      
     }
 }
diff --git a/src/GeneticSharp.Domain.UnitTests/Selections/SelectionIssuesTest.cs b/src/GeneticSharp.Domain.UnitTests/Selections/SelectionIssuesTest.cs
@@ -0,0 +1,73 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using NSubstitute;
+using GeneticSharp.Extensions;
+
+namespace GeneticSharp.Domain.UnitTests.Selections
+{
+    [TestFixture()]
+    [Category("Selections")]
+    public class SelectionIssuesTest
+    {
+        [SetUp]
+        public void Cleanup()
+        {
+            RandomizationProvider.Current = new BasicRandomization();
+        }      
+
+        /// <summary>
+        /// https://github.com/giacomelli/GeneticSharp/issues/92
+        /// </summary>
+        [Test]
+        [TestCase("Rank")]
+        [TestCase("Roulette Wheel")]
+        [TestCase("Tournament")]
+        public void SelectChromosomes_Issue92_Solved(string selectionName)
+        {
+            var target = SelectionService.CreateSelectionByName(selectionName);
+            var c1 = new SelectionStubChromosome();
+            c1.Fitness = 1;
+
+            var c2 = new SelectionStubChromosome();
+            var c3 = new SelectionStubChromosome();
+            var c4 = new SelectionStubChromosome();
+
+            var generation = new Generation(1, new List<IChromosome>() {
+                c1, c2, c3, c4
+            });
+
+            var actual = target.SelectChromosomes(10, generation);
+            Assert.AreEqual(10, actual.Count);
+
+            var previousChromosomes = actual.Select(c => c.GetGenes().ToArray()).ToArray();
+            var mutation = new UniformMutation(true);
+
+
+            for (int i = 0; i < actual.Count; i++)
+            {
+                if (actual[i].Fitness == 1)
+                {
+                    mutation.Mutate(actual[i], 1);
+
+                    Assert.AreEqual(1, actual.Count(c => c.GetGene(0).Value != null), "Mutation has changed more than one chromosome at the time");
+                    break;
+                }
+            }
+
+            for (int i = 0; i < actual.Count; i++)
+            {
+                for (int j = 0; j < actual.Count; j++)
+                {
+                    if (i == j)
+                        continue;
+
+                    if (object.ReferenceEquals(actual[i], actual[j]))
+                        Assert.Fail($"Chromosomes on index {i} and {j} are the same.");
+                }
+            }
+        }
+    }
+}
+
diff --git a/src/GeneticSharp.Domain.UnitTests/Selections/SelectionStubChromosome.cs b/src/GeneticSharp.Domain.UnitTests/Selections/SelectionStubChromosome.cs
@@ -0,0 +1,20 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace GeneticSharp.Domain.UnitTests.Selections
+{
+    class SelectionStubChromosome : ChromosomeBase
+    {
+        public SelectionStubChromosome() : base(2)
+        {
+            Fitness = 0;
+        }
+
+        public override IChromosome CreateNew() => new SelectionStubChromosome();
+
+        public override Gene GenerateGene(int geneIndex) => new Gene(0);
+    }
+}
diff --git a/src/GeneticSharp.Domain/Chromosomes/ChromosomeBase.cs b/src/GeneticSharp.Domain/Chromosomes/ChromosomeBase.cs
@@ -187,19 +187,7 @@ public void ReplaceGenes(int startIndex, Gene[] genes)
                     throw new ArgumentOutOfRangeException(nameof(startIndex), "There is no Gene on index {0} to be replaced.".With(startIndex));
                 }
 
-                var genesToBeReplacedLength = genes.Length;
-
-                var availableSpaceLength = m_length - startIndex;
-
-                if (genesToBeReplacedLength > availableSpaceLength)
-                {
-                    throw new ArgumentException(
-                        nameof(Gene),
-                        "The number of genes to be replaced is greater than available space, there is {0} genes between the index {1} and the end of chromosome, but there is {2} genes to be replaced."
-                        .With(availableSpaceLength, startIndex, genesToBeReplacedLength));
-                }
-
-                Array.Copy(genes, 0, m_genes, startIndex, genes.Length);
+                Array.Copy(genes, 0, m_genes, startIndex, Math.Min(genes.Length, m_length - startIndex));
 
                 Fitness = null;
             }

diff --git a/src/GeneticSharp.Domain/Selections/RankSelection.cs b/src/GeneticSharp.Domain/Selections/RankSelection.cs
@@ -59,7 +59,7 @@ protected static IList<IChromosome> SelectFromWheel(int number, IList<IChromosom
                                         .FirstOrDefault(r => r.Value >= pointer);
 
                 if (chromosome != null)
-                    selected.Add(chromosomes[chromosome.Index]);
+                    selected.Add(chromosomes[chromosome.Index].Clone());
             }
 
             return selected;

diff --git a/src/GeneticSharp.Domain/Selections/RouletteWheelSelection.cs b/src/GeneticSharp.Domain/Selections/RouletteWheelSelection.cs
@@ -26,16 +26,13 @@ namespace GeneticSharp
     [DisplayName("Roulette Wheel")]
     public class RouletteWheelSelection : SelectionBase
     {
-        #region Constructors
         /// <summary>
         /// Initializes a new instance of the <see cref="GeneticSharp.Domain.Selections.RouletteWheelSelection"/> class.
         /// </summary>
         public RouletteWheelSelection() : base(2)
         {
         }
-        #endregion
 
-        #region ISelection implementation
         /// <summary>
         /// Selects from wheel.
         /// </summary>
@@ -57,7 +54,7 @@ protected static IList<IChromosome> SelectFromWheel(int number, IList<IChromosom
                                         .FirstOrDefault(r => r.Value >= pointer);
 
                 if (chromosome != null)
-                    selected.Add(chromosomes[chromosome.Index]);
+                    selected.Add(chromosomes[chromosome.Index].Clone());
             }
 
             return selected;
@@ -96,7 +93,6 @@ protected override IList<IChromosome> PerformSelectChromosomes(int number, Gener
             CalculateCumulativePercentFitness(chromosomes, rouletteWheel);
 
             return SelectFromWheel(number, chromosomes, rouletteWheel, () => rnd.GetDouble());
-        }
-        #endregion
+        }        
     }
 }
diff --git a/src/GeneticSharp.Domain/Selections/TournamentSelection.cs b/src/GeneticSharp.Domain/Selections/TournamentSelection.cs
@@ -15,7 +15,6 @@ namespace GeneticSharp
     [DisplayName("Tournament")]
     public class TournamentSelection : SelectionBase
     {
-        #region Constructors
         /// <summary>
         /// Initializes a new instance of the <see cref="TournamentSelection"/> class.
         /// <remarks>
@@ -48,9 +47,7 @@ public TournamentSelection(int size, bool allowWinnerCompeteNextTournament) : ba
             Size = size;
             AllowWinnerCompeteNextTournament = allowWinnerCompeteNextTournament;
         }
-        #endregion
-
-        #region Properties
+
         /// <summary>
         /// Gets or sets the size of the tournament.
         /// <remarks>
@@ -66,9 +63,8 @@ public TournamentSelection(int size, bool allowWinnerCompeteNextTournament) : ba
         /// </remarks>
         /// </summary>
         public bool AllowWinnerCompeteNextTournament { get; set; }
-        #endregion
+
 
-        #region Methods
         /// <summary>
         /// Performs the selection of chromosomes from the generation specified.
         /// </summary>
@@ -94,7 +90,7 @@ protected override IList<IChromosome> PerformSelectChromosomes(int number, Gener
                 var randomIndexes = RandomizationProvider.Current.GetUniqueInts(Size, 0, candidates.Count);
                 var tournamentWinner = candidates.Where((c, i) => randomIndexes.Contains(i)).OrderByDescending(c => c.Fitness).First();
 
-                selected.Add(tournamentWinner);
+                selected.Add(tournamentWinner.Clone());
 
                 if (!AllowWinnerCompeteNextTournament)
                 {
@@ -103,7 +99,6 @@ protected override IList<IChromosome> PerformSelectChromosomes(int number, Gener
             }
 
             return selected;
-        }
-        #endregion
+        }        
     }
 }