[R-package] added support for first_metric_only (fixes #2368)

[jameslamb]

This PR attempts to add support for first_metric_only in the R package.

This got complicated because of some inconsistencies in the way we describe what you can specify for eval. From the existing docs and from the Python side (#2049, #2209), I think we want to support the following combinations:

• eval
  • one function
  • character vector
  • list of strings
  • list of functions
  • list with mix of strings and functions
  • NULL or empty list to say "no additional evaluation metrics"
• metric
  • string "None" to say "no metric"
  • character vector with one or more strings

...where "string" or "character" stuff in that list refers to these metrics which are known to LightGBM and computed on the C++ side (https://lightgbm.readthedocs.io/en/latest/Parameters.html#metric)

I believe I've accomplished that in this PR. At this point I'm far enough along that I could use a review.

I know there are some items that still need to be addressed, but want to wait for review comments before going too much farther and growing the diff.

• similar changes to lightgbm.cv() + tests on lightgbm.cv()
• make sure docs on eval are consistent across lgb.train(), lightgbm(), and lgb.cv()

NOTE: I fixed a bug in lgb.params2string() along the way here. I think I'll move that to a different PR, so it doesn't get stuck in this review and so this diff gets a bit smaller.

[guolinke]

I think we should only use one label in https://github.com/microsoft/LightGBM/blob/master/.github/release-drafter.yml

[jameslamb]

I think we should only use one label in https://github.com/microsoft/LightGBM/blob/master/.github/release-drafter.yml

ah ok yep, I agree! Still getting used to the release drafter. Thanks!

[jameslamb]

I am going to get this PR updated this week. I want to get it and a few others into 3.0.0 (#3071 ) and I know we want to do a release as soon as possible.

I'll fix the merge conflicts tonight and update it to master, and then let reviewers know when it is ready for another review (some time in the next few days)

[jameslamb]

ok I think this is ready for a review!

This is ready to be reviewed, but since it is a new feature, this PR should not be merged until after 3.0.0 is released (#3293 )

[guolinke]

@jameslamb It seems R's tests often failed, is that normal?

[jameslamb]

@jameslamb It seems R's tests often failed, is that normal?

yeah unfortunately it's because there isn't a proper package manager for some of the artifacts we download, like Rtools and MiKTeX. So you see a lot of issues like this:

I know @StrikerRUS has mentioned it in the past...is there a place we could self-host these that might be more reliable?

At least for things that are never changing (like Rtools35.exe) or things where we don't care about new versions (like miktex-setup.exe), that might provide a bit more stability. For Rtools40.exe, it is not stable yet so we need to keep pulling the newest version.

[StrikerRUS]

@jameslamb

is there a place we could self-host these that might be more reliable?

Maybe our GitHub Releases page?

LightGBM/.ci/setup.sh

Line 31 in ca066d4

wget -q https://github.com/microsoft/LightGBM/releases/download/v2.0.12/AMD-APP-SDKInstaller-v3.0.130.136-GA-linux64.tar.bz2

[jameslamb]

is it ok if I host things directly on that specific release (2.0.12)?

I don't think it will be too confusing for users, since that is such an old release. And that seems like the easiest free option.

Otherwise, maybe Microsoft could cover the costs of a small Azure Blob Store container for this project's CI.

[guolinke]

@jameslamb
I think using azure blob is not a problem.
for gihub release, maybe we can create a special tag to host these files.

[StrikerRUS]

@jameslamb

I don't think it will be too confusing for users, since that is such an old release. And that seems like the easiest free option.

Yeah, I believe old releases are perfect places to store some files required for our CI.

[jameslamb]

ok added #3359 with these artifacts, let's move the conversation there

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.