lballabio / QuantLib

The QuantLib C++ library
http://quantlib.org
Other
5.21k stars 1.78k forks source link

Duplicate arguments in LevenbergMarquardt optimization method #2047

Open eltoder opened 1 month ago

eltoder commented 1 month ago

LevenbergMarquardt currently accepts xtol and gtol arguments in the constructor. These are passed as the respective arguments into the MINPACK::lmdif() call. These appear to be duplicates of rootEpsilon and gradientNormEpsilon values already present on the EndCriteria argument. Consequently, the values from the EndCriteria are ignored. They still have to be passed to the EndCriteria constructor though, since there are no defaults. This seems a bit confusing. Should the xtol and gtol arguments be removed and the EndCriteria values used instead?

eltoder commented 1 month ago

I guess it can also be done the other way around: remove these values from EndCriteria or maybe even remove EndCriteria altogether. Put all "maximum iterations" and "tolerances" arguments on the optimization methods. This has two advantages:

  1. It allows users to configure the optimization method and pass it as a single parameter into the code that needs it (instead of having to pass multiple parameters).
  2. Each optimization method will only expose options that it uses, instead of having a superset of all possible options on EndCriteria.
lballabio commented 1 month ago

Hmm, you're right, it is confusing. The problem is, EndCriteria is a bother to instantiate with just the parameters for a given optimizer (at least until we can use designated initializers) but is in the signature of the parent Optimizer class so it can't be removed from just Levenberg-Marquardt, and it's difficult to deprecate correctly. On the other hand, removing xtol and gtol would leave us with half the parameters in EndCriteria and the other half in the constructor. I'm not sure what steps to take here.