You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Sklearn's coordinate descent solver computes and uses the dual_gap as part of its stopping criteria, while ours does not. This causes a few problems:
In the cuml.accel layer, our exposed estimators lack a dual_gap_ fitted attribute, exposing the difference between the primary and the dual as a result of the fit. We tried to patch around this in Support dual_gap_ on ElasticNet & Lasso #6714, but it came with a performance cost and didn't make sense if the computed value wasn't being used in the solver.
The meaning of the tol parameter differs between solvers. Currently we work around that by scaling the tol parameter when converting to/from sklearn's parameters. If our solver's had similar stopping criteria we wouldn't need to do this, and could better guarantee functionally equivalent results.
Since we'll need to look into the solver anyway in #6736, I think we should consider making this change to better improve compatibility with sklearn's ElasticNet/Lasso implementations.
Sklearn's coordinate descent solver computes and uses the
dual_gapas part of its stopping criteria, while ours does not. This causes a few problems:cuml.accellayer, our exposed estimators lack adual_gap_fitted attribute, exposing the difference between the primary and the dual as a result of the fit. We tried to patch around this in Supportdual_gap_onElasticNet&Lasso#6714, but it came with a performance cost and didn't make sense if the computed value wasn't being used in the solver.tolparameter differs between solvers. Currently we work around that by scaling thetolparameter when converting to/from sklearn's parameters. If our solver's had similar stopping criteria we wouldn't need to do this, and could better guarantee functionally equivalent results.Since we'll need to look into the solver anyway in #6736, I think we should consider making this change to better improve compatibility with sklearn's
ElasticNet/Lassoimplementations.