Hello.
> > > [...]
(1)
o.a.c.m.ga.GeneticAlgorithmTestPermutations
(under "src/test")
As per your comment in that class, it is a usage example.
Given that its name does not end with "Test", it is not run by the
test suite. Please move it to the "examples" module.
(2)
I'm missing a high-level doc that would enable a newbie to figure
out what to implement in order to get going.
E.g. what is the interplay between
* genotype
* allele
* phenotype
* decoder
* fitness function
?
Several classes do not provide explanations (or links) about the
concept which they represent. For example, there is no doc about
what a "RandomKeyDecoder" is, and the reason for using it (or not).
(3)
o.a.c.m.ga.utils.ChromosomeRepresentationUtils
It seems to be a "mixed-bag" kind of class (that is being frowned
upon nowadays).
Its comment refers to "random" but some methods are not using
any randomization. Most methods are only used in unit tests.
(4)
o.a.c.m.ga.RandomProviderManager
As already discussed, this class should not be part of the public
API, namely because the "getRandomProvider()" method returns
an object that is not thread-safe.
If used internally as "syntactic sugar", it should be located in a
package named "internal"; however I'd tend to remove it
altogether, and call "ThreadLocalRandomSource.current(...)"
explicitly.
(5)
Why does a "Chromosome" need an "identifier"?
Method "getId()" is only used in "PopulationStatisticalSummaryImpl"
that is an internal class, where it seems that the chromosome itself
(rather than its "id") could serve as the map's key.
(6)
o.a.c.m.ga.chromsome.AbstractChromosome
Field "fitness" is not "final", yet it could be: a "FitnessFunction"
object (used in "evaluate() to compute that field) is passed to the
constructor. Is there a reason for the "lazy" evaluation?
Dropping it would make the instance immutable (and "evaluate()"
should be renamed to "getFitness()").
Why should the "FitnessFunction" be stored in every chromosome?
(7)
Spurious "@since" tags: In the new code (in "commons-math-ga"
module), none should refer to a version < 4.0.
(8)
@SuppressWarnings("unchecked")
By default, I'm a bit suspicious about having to resort to these annotations,
especially for the kind of algorithms we are trying to implement.
What do you think of the alternative approach outlined in the ZIP file
attached in MATH-1618:
https://issues.apache.org/jira/browse/MATH-1618
?
(9)
Naming of factory methods should be harmonized to match the convention
adopted in components like [RNG] and [Numbers].
E.g. instead of "newChromosome(...)", please use "of(...)" or "from(...)"
for "value object", and "create(...)" otherwise.
(10)
o.a.c.m.ga.chromosome.AbstractListChromosome
Constructor is called with an argument that is a previously instantiated
"representation". If the latter is mutable, the caller will be able to modify
the underlying data structure of the newly created chromosome. [The
doc assumes immutability of the representation but this cannot be
enforced, and mixed ownership can entail subtle bugs.]
(11)
Do we agree that, in a GA, the most time-consuming task is the fitness
computation? Hence IMO, it should be the focus of the multithreading
tools (i.e. "ExecutorService"), probably keeping the other parts (namely
the genetic operators) within a simple sequential loop (as in class
"GeneticAlgorithmFactory" in MATH-1618).
To be continued...
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
