Hello, this looks like a promising package! Here are random remarks I got while running the tuto with Aymeric.
- In CI, don't comment the pep8 test 馃槢
- In CI, run the tuto notebook to make sure this is not broken with new changes.
- it can be anoying for import to have capitalize name
PEPit, using pepit avoid having to remember what is capital and what is not.
- I was surprised that
SmoothStronglyConvexFunction does not raise a ValueError when given mu=0. Using an internal base class wth clearly split cases would avoid user shooting them selfves in the foot.
-
PEP.set_initial_point: in term of API, it is weird to call a set_* method that do not take any argument. Would call this create_initial_point or get_initial_point.
- In tutorial, section
Algorithm Implementation, iit would be nice to define n here.
- In the tuto, section
Solving the PEP, you could rewrite the equation $|f(x_t) - f(x*)| \le \tau |x_0 - x*|$. It would help to compare with the equation bellow, saying $\tau = L / 4 ...$ .
- In tuto
Comparison of analytical (obtained in the literature) ... could be move to a follow up notebook to avoid scaring people (stuff after conclusion is always scary :) )
Hello, this looks like a promising package! Here are random remarks I got while running the tuto with Aymeric.
PEPit, usingpepitavoid having to remember what is capital and what is not.SmoothStronglyConvexFunctiondoes not raise aValueErrorwhen givenmu=0. Using an internal base class wth clearly split cases would avoid user shooting them selfves in the foot.PEP.set_initial_point: in term of API, it is weird to call aset_*method that do not take any argument. Would call thiscreate_initial_pointorget_initial_point.Algorithm Implementation, iit would be nice to definenhere.Solving the PEP, you could rewrite the equationComparison of analytical (obtained in the literature) ...could be move to a follow up notebook to avoid scaring people (stuff after conclusion is always scary :) )