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 :stuck_out_tongue:
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
, usingpepit
avoid having to remember what is capital and what is not.SmoothStronglyConvexFunction
does not raise aValueError
when 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_point
orget_initial_point
.Algorithm Implementation
, iit would be nice to definen
here.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 ...$ .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 :) )