Codecov Report

Base: 96.27% // Head: 97.72% // Increases project coverage by +1.45% 🎉

Coverage data is based on head (c903187) compared to base (994fe73).
Patch coverage: 100.00% of modified lines in pull request are covered.

@@            Coverage Diff             @@
##           master     #477      +/-   ##
==========================================
+ Coverage   96.27%   97.72%   +1.45%     
==========================================
  Files          27        7      -20     
  Lines        2977      968    -2009     
==========================================
- Hits         2866      946    -1920     
+ Misses        111       22      -89     

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Besides, I think ic_type can be renamed as eval_type because this argument is not only for IC now.
And shall we add an argument to specify OvO/OvR for MultinomialRegression?

@oooo26 ，shall we change ic_type to eval_type?

@oooo26 ，shall we change ic_type to eval_type?

may be other. you can search scikit-learn for reference

may be other. you can search scikit-learn for reference

Oh Yes. They have a scoring argument in LogisticRegressionCV to control it, but not in the non-CV one. They don't seem to offer different IC type for classification too, only for Lasso.

Shall we add a cv_score or something? (Only scoring might be misunderstood since our regressor combines CV and non-CV problem.)

And I think ic_type/cv_score can share the same API in Cpp (that's what I have done so far), since only one of the criteria will be used.

two excellent comments. just do it.

@EQUIWDH you can inspect cpp code and program for cindex similarly.

two excellent comments. just do it.

OK! I will update soon.

And @EQUIWDH, if you would like to add other loss like c-index for CV, please see the test_loss() function in Metric.h. (Other files changed are just trivia.)

Add cv_score:

• cv_score='test_loss': default;
• cv_score='roc_auc': additional for logistic;
• cv_score='roc_auc_ovo' or 'roc_auc_ovr': additional for multinomial;

Since AUC score seems to perform better on Logistic (both speed and accuracy), maybe we can set LogisticRegression(cv_score='roc_auc') as default?

Oh there is a small conflict on R: when using CV, the ic_type should be set as 0 so that it can use test loss as before.

But currently, it is 1. We may need to change it in R-package/R/utility.R. (Otherwise, there will be some annoying warnings...)

I skim the R code in R-package now, and I think @bbayukari can quickly address this remaining warning in abesspca (see https://github.com/abess-team/abess/actions/runs/4044489654/jobs/6954695218).

I skim the R code in R-package now, and I think @bbayukari can quickly address this remaining warning in abesspca (see https://github.com/abess-team/abess/actions/runs/4044489654/jobs/6954695218).

Yep, just setting ic_type=0 under CV will fix it. Because ic_type=1 will represent AUC score for logistic now.

I skim the R code in R-package now, and I think @bbayukari can quickly address this remaining warning in abesspca (see https://github.com/abess-team/abess/actions/runs/4044489654/jobs/6954695218).

Yep, just setting ic_type=0 under CV will fix it. Because ic_type=1 will represent AUC score for logistic now.

Seems everything is fine now! You can merge it if you also believe this is fine.