Conversation
Application of genetic algorithms to optimize RFID antenna readings
Merge from Root
The Method of Studing of Electromagnetic Characteristics and Synthesis of Metamaterial"
Fix incorrect IPopulation link in README
Merge fork 11/2020
Sudoku Optimizations
Merge from upstream
…n't need initialization, which can be a costly operation depending on the chromosome)
…heuristics based ordered sub chromosomes crossovers
| /// </remarks> | ||
| /// </summary> | ||
| protected virtual void CreateGenes() | ||
| public virtual void CreateGenes() |
There was a problem hiding this comment.
Just a reminder:
Because of this change, when a release is published, we will have to increase the major version to 3, as it is a change in the public API and will affect projects that overrides this method when it was protected.
There was a problem hiding this comment.
See my other comment. The new proposal is safer I suppose, and could potentially trigger only a minor version bump, but I think a Major version increase is still probably a good idea because of the new CreateNew + InitializeGenes protocol.
| /// <summary> | ||
| /// Initializes a chromosome with new random genes | ||
| /// </summary> | ||
| void CreateGenes(); |
There was a problem hiding this comment.
This is another change that requires us to increase the major version.
There was a problem hiding this comment.
Good call, with a second thought, that was actually uncalled for. I just commited an alternative, where the new interface method has another name and the historical protected one is left untouched with the new method rerouted to the former one.
This may seem a bit weird, and I think that this kind of change probably still justifies bumping the major version anyway, since it cTor-based initialization should be considered bad practice from now on, but at least only derived Chromosome classes implementing the Interface directly would see a breaking change, which is most probably none of them.
Anyway, I guess we could have lived without that specific change, but I figured, since this was always going to be a problem down the road, it is now a good moment to start paying the technical debt.
|
Closing due to inactivity. Feel free to fix the CI errors and reopen the PR. |
This pull request introduces an important change: it decouples chromosome creation and gene initialisation. As I understand, that was always the original intent, with for instance the IPopulation interface and TplPopulation implementation entirely based on the premise that those two should be decouples, but in practice, many chromosomes required to have a global context for individual genes creation (such as the TSP chromosome). Accordingly, it was tempting to initialise the genes within the constructor, which was mostly the case.
That in turn defeated entirely the original intent, because that meant every CreateNew call from a Crossover would trigger unneeded gene initialization with potentially great penalty costs.
The new proper way to do global context-based initialization rather than individually through the usual GenerateGene method is through overriding the CreateGenes method from the base class.
I went through all existing code to update the new behavior. Note that it shouldn't break things if a chromosome does early gene intialisation in the constructor. It just does not have to do it anymore, and shouldn't for better performances, since CreateNew will be explicitly called on each chromosome during first generation initialization, yielding dual initialisation if that's the case.