CMA-ES / pycma

Python implementation of CMA-ES
Other
1.1k stars 178 forks source link

Clarify documentation of Augmented Lagrangian initial coefficients settings #197

Open paulduf opened 2 years ago

paulduf commented 2 years ago

I have a few comments to make regarding the docstring of AugmentedLagrangian.set_coefficients

1. Error in the docstring

The docstring relates to the following formula for setting the lagrangian parameter:

lam = iqr(f) / (sqrt(n) * iqr(g))

whereas, in the code, I can read

self.lam[idx_inequ] = df / (self.dimension * dG[idx_inequ] + 1e-11 * (df + 1))

It looks like the new default is dimension instead of sqrt(dimension)

2. What does this mean ?

At the end of the docstring I can read:


""""Set lam and mu until a population contains more than 10% infeasible
and more than 10% feasible at the same time. Afterwards, this at least...?.. """.```

But I don't find the corresponding piece of code ?

3.

My last request for documentation is about this code

# take min or max with existing value depending on the sign average
            # we don't know whether this is necessary
            isclose = np.abs(sign_average) <= 0.2
            idx1 = _and(_and(_or(isclose,
                                 _and(_not(self.isequality), sign_average <= 0.2)),
                             _or(self.mu == 0, self.mu > mu_new)),
                             idx)  # only decrease
            idx2 = _and(_and(_and(_not(isclose),
                                  _or(self.isequality, sign_average > 0.2)),
                                  self.mu < mu_new),
                                  idx)  # only increase
            idx3 = _and(idx, _not(_or(idx1, idx2)))  # others
            assert np.sum(_and(idx1, idx2)) == 0
            if np.any(idx1):  # decrease
                iidx1 = self.mu[idx1] > 0
                if np.any(iidx1):
                    self.lam[idx1][iidx1] *= mu_new[idx1][iidx1] / self.mu[idx1][iidx1]
                self.mu[idx1] = mu_new[idx1]
            if np.any(idx2):  # increase
                self.mu[idx2] = mu_new[idx2]
            if np.any(idx3):
                self.mu[idx3] = mu_new[idx3]

Which is hard to read and looks somewhat related to my 2nd point. This looks like this is to decide whether mu_new should be chosen as the new setting if the user has already set values for mu, based on the value of sign_average and other conditions. So does it relate to the quoted part of the docstring and should it be updated ?

nikohansen commented 11 months ago
  1. fixed with the next push.
  2. sign_average < -0.8 means less than 10% are infeasible, but I also suspect that the text is not accurate.
  3. at some point to be reviewed/revised