Conversation
Ulfgard
left a comment
There was a problem hiding this comment.
i think the code is not as unified as it could be. but my comments should make it a lot easier already. But i have the feeling that we can unify both cases just by setting the negative weights to 0 if no active updates are needed.
|
|
||
| bool m_userSetMu; /// <The user set a value via setMu, do not overwrite with default | ||
| bool m_userSetLambda; /// <The user set a value via setMu, do not overwrite with default | ||
| bool m_useActiveUpdates = false; /// <Indicates if active updates should be applied |
There was a problem hiding this comment.
please put that in the constructor, everything should be at one place.
| } | ||
| }; | ||
|
|
||
| ///\brief Reverse ordering relation by the fitness of the individuals(only single objective) |
There was a problem hiding this comment.
you could use a lambda function for that, we are trying to replace all those small functors with them.
| for (std::size_t i = 0; i < m_mu; i++) | ||
| for (std::size_t i = 0; i < m_mu; i++) { | ||
| m_weights(i) = (double)(mu-i); | ||
| negativeWeights(i) = static_cast<double>(mu - (m_lambda - i - 1.)); |
There was a problem hiding this comment.
you should not need the static_cast because subtracting a double from mu leads to a double, unlike the line before where an unsigned integral type is assigned to double.
| double alphaMu = 2.; | ||
| double rankMuAlpha = 0.3;//but is it really? | ||
| m_cMu = std::min(1. - m_c1, alphaMu * ( rankMuAlpha + m_muEff - 2. + 1./m_muEff) / (sqr(m_numberOfVariables + 2) + alphaMu * m_muEff / 2)); // eq. (49) | ||
| m_cMu = std::min(1. - m_c1, 2. * (.25 + m_muEff + 1. / m_muEff - 2.) / (std::pow(m_numberOfVariables + 2., 2.) + 2. * m_muEff / 2.)); // eq. (49) |
There was a problem hiding this comment.
replace std::pow(a,2.0) by sqr(a)
also this line should be the exact same as before when substitution alphaMu by 2 and changing rankMuAlpha to 0.25
| row(vectors, weightIndex) = normalized; | ||
| } | ||
|
|
||
| m_evolutionPathC = (1. - m_cC) * m_evolutionPathC + hSig * (std::sqrt(m_cC * (2. - m_cC) * m_muEff) / m_sigma) * (m - m_mean); |
There was a problem hiding this comment.
this is the same as linke 382. (m-mean)/sigma is y. Please try to unify!
There was a problem hiding this comment.
Indeed it should, I did have some weird error when I did it with y instead, but that might origin from somewhere else. I will try to update it!
|
|
||
| m_evolutionPathC = (1. - m_cC ) * m_evolutionPathC + hSig * std::sqrt( m_cC * (2. - m_cC) * m_muEff ) * y; // eq. (42) | ||
| noalias(C) = (1.-m_c1 - m_cMu) * C + m_c1 * ( blas::outer_prod( m_evolutionPathC, m_evolutionPathC ) + deltaHSig * C) + m_cMu * 1./sqr( m_sigma ) * Z; // eq. (43) | ||
| const double c1a = m_c1 * (1. - (1. - (hSig * hSig)) * m_cC * (2. - m_cC)); |
There was a problem hiding this comment.
please move inside the if, if you don't use it in both branches (or later)
|
|
||
| if (tempWeights[weightIndex] < 0.) | ||
| { | ||
| const double mahalanobisNorm = sqrt(sum(sqr((trans(B) % normalized) / D))); |
There was a problem hiding this comment.
const double mahalanobisNorm = norm_2((trans(B) % normalized) / D)
or even simpler
const double mahalanobisNorm = norm_2(selectedOffspring[j].chromosome())
| tempWeights[weightIndex] *= static_cast<double>(m_numberOfVariables) / sqr(mahalanobisNorm + 1e-9); | ||
| } | ||
|
|
||
| row(vectors, weightIndex) = normalized; |
There was a problem hiding this comment.
noalias(row(vectors,weightIndex)) = normalized;
we don't want to copy!
This PR provides active updates in the CMA-ES algorithm.
The code is implemented and test with reference to the pycma code found at https://github.com/CMA-ES/pycma