Fixed some bugs. Still thinking about Animal Tree. #6
Conversation
|
|
||
|
|
||
| private Map<Genotype, Integer> genotypeCount = new HashMap<>(); | ||
| private PriorityQueue<Map.Entry<Genotype, Integer>> maxHeap = new PriorityQueue<>(Comparator.comparingInt(entry -> ((Map.Entry<Genotype, Integer>) entry).getValue()).reversed()); |
There was a problem hiding this comment.
i was wondering if genotypeCount is still needed. We could technically keep the counter inside of the heap, but the hashmap allows to instantly know where certain genotype is in a heap. With just pairs of <genotype, counter> in the heap, we would have to look for the genotype throught the whole heap O(n)
There was a problem hiding this comment.
If we want to update value in a PriorityQueue we have to remove() this element and then add() it with updated value.
As far as I am concerned there is no way to force heapify in PriorityQueue.
If we want to optimize it we can make our own heap implementation.
If not keeping Map simply adds unnecessary complexity to our code.
| @@ -72,9 +73,4 @@ public Genotype getGenotype() { | |||
| return genotype; | |||
There was a problem hiding this comment.
If we return a genotype object it becomes modifiable.
Not sure if that is what you want.
Tbh idk if this is a right approach.
|
|
||
| for (int i = 0; i < numberOfMutations; i++) { | ||
| switchGenes(RandomInteger.getRandomInt(genes.size()), RandomInteger.getRandomInt(genes.size())); //opcja symulacji z podmianką | ||
| randomGene(RandomInteger.getRandomInt(genes.size())); //opcja symulacji pełna losowość | ||
| switchGenes(RandomInteger.getRandomInt(genes.size()-1), RandomInteger.getRandomInt(genes.size()-1)); //opcja symulacji z podmianką |
There was a problem hiding this comment.
In the future we should separate those functions because we want to use only one mutation at a time.
|
|
||
|
|
||
| private Map<Genotype, Integer> genotypeCount = new HashMap<>(); | ||
| private PriorityQueue<Map.Entry<Genotype, Integer>> maxHeap = new PriorityQueue<>(Comparator.comparingInt(entry -> ((Map.Entry<Genotype, Integer>) entry).getValue()).reversed()); |
There was a problem hiding this comment.
If we want to update value in a PriorityQueue we have to remove() this element and then add() it with updated value.
As far as I am concerned there is no way to force heapify in PriorityQueue.
If we want to optimize it we can make our own heap implementation.
If not keeping Map simply adds unnecessary complexity to our code.
| private final List<Integer> genes = new ArrayList<>(); | ||
| private int minNumberOfMutations; | ||
| private int maxNumberOfMutations; | ||
| private List<Integer> genes = new ArrayList<>(); |
There was a problem hiding this comment.
I believe that it still should be final.
You can do it like that:
private final List<Integer> genes; - line 11
genes = new ArrayList<>(genotypeLength); - new line in non-copying constructor
| @@ -32,20 +32,6 @@ public void testSwitchGenes() { | |||
| assertEquals(initialFirstGene, genotype.getCurrentGene(genotypeLength - 1)); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testRandomGene() { | |||
There was a problem hiding this comment.
You can keep it and make it probabilistic test by modyfing all 10 genomes.
Then the chance of failure will only be (1/8)^10.
I would increase the length of genotype to 20/30 then the chance of this test failing will be ridiculously low.
To check whole genotype easily you can compare them with assertEquals() and toString().
There was a problem hiding this comment.
@Test
public void testRandomGene() {
int genotypeLength = 30;
Genotype genotype = new Genotype(genotypeLength, 1, 3);
String initialGenotype = genotype.toString();
for(int i = 0; i < genotypeLength; i++) {
genotype.randomGene(i);
}
assertNotEquals(initialGenotype, genotype.toString());
}
Made the changes that I have mentioned during latest code review. Fixed some bugs. Improved the GenotypeTests.
|
@Gabriela-Dumanska before commiting make sure to run the tests. Also try to write more tests. It will make it easier to catch any potential bugs. |
No description provided.