Create common base class for optimizer classes. #375
Conversation
bwohlberg
added
the
improvement
Codecov Report
@@ Coverage Diff @@
## main #375 +/- ##
==========================================
- Coverage 94.08% 93.99% -0.10%
==========================================
Files 61 61
Lines 3821 3747 -74
==========================================
- Hits 3595 3522 -73
+ Misses 226 225 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| Args: | ||
| f: Loss or Functional object with `grad` defined. | ||
| g: Instance of Functional with defined prox method. | ||
| L0: Initial estimate of Lipschitz constant of f. | ||
| x0: Starting point for :math:`\mb{x}`. | ||
| step_size: helper :class:`StepSize` to estimate the Lipschitz | ||
| constant of f. | ||
| maxiter: Maximum number of PGM iterations to perform. |
There was a problem hiding this comment.
I miss seeing maxiter listed in the docs for each optimizer. What do you think about putting it back (while still refactoring out itstat_options)? Or maybe mention it in the help string for **kwargs?
There was a problem hiding this comment.
I see arguments for an against.
For: It would improve readability of individual optimizer documentation, and is a relatively small change.
Against: One of the reasons for use of **kwargs and moving the docs for those options to the base class is to avoid duplication of docs, which inevitably leads to errors when changes are made. Keeping the maxiter docs in the derived classes would be inconsistent with this principle.
Thoughts?
There was a problem hiding this comment.
It's a tough call. I value the readability of the docs over the maintenance issue, but not strongly. And, the PR already is as it is, so perhaps we leave it.
There was a problem hiding this comment.
Let's go with the PR as-is. We can make the suggested changes later if we decide it's a serious issue.
Create common base class for optimizer classes. A significant fraction of the iterations stats initialization code is moved to the base class.
In addition, fix some faulty logic in the determination of whether an objective function can be evaluated.