[ci][fix] Fix cuda_exp ci

[shiyu1994]

Unfortunately, in a very long time (since the merge of #4630), the ci tests for cuda_exp has not actually run.
Since the task name for both cuda and cuda_exp is cuda

LightGBM/.github/workflows/cuda.yml

Line 14 in 702db13

task: cuda

in test.sh the cuda_exp option cannot be detected, and it will always run for device cuda!

LightGBM/.ci/test.sh

Lines 198 to 199 in 702db13

	elif [[ $TASK == "cuda" || $TASK == "cuda_exp" ]]; then
	if [[ $TASK == "cuda" ]]; then

That's a serious mistake. And I think we need to fix it right now for our on-going development of cuda_exp.

[shiyu1994]

See #5425 (comment)

[jameslamb]

Thank you very much for noticing and proposing a fix for this! But I think there's a much simpler fix that will achieve the same goal.

I think doing the following would prevent adding new complexity to .ci/test.sh:

1. replace treelearner: in .github/workflows/cuda.yml with task:
2. remove this line:

   LightGBM/.github/workflows/cuda.yml

   Line 14 in 702db13

   task: cuda

3. change the following line to TASK="${{ matrix.task }}":

   LightGBM/.github/workflows/cuda.yml

   Line 89 in 702db13

   TASK=${{ env.task }}

I remember we had a discussion in #4630 about not adding more complexity to test.sh: #4630 (comment).

---

In addition to that request...right now, both cuda_exp CI jobs are failing with the following error

Fatal Python error: Aborted

(build link)

We should never merge a PR that will result in CI being broken on master, so please do one of these:

• push a fix for that issue to this PR
• remove the cuda_exp CI jobs in this PR, document the work to add them back in a separate issue, merge this PR, and then push a follow-up PR fixing cuda_exp and adding back the CI jobs

I hope a fix can just be pushed here, but I don't know how complicated it will be to debug this. If it is very complicated, then we might as well eliminate those two cuda_exp jobs that are not actually testing cuda_exp, to save CI time.

And if the "make an issue and fix this later" approach is taken, then I think we should not merge any more cuda_exp PRs until those CI jobs have been re-enabled.

[jameslamb]

I've added the label maintenance to this PR.

As I mentioned in #5403 (comment) and #5413 (comment), please add a labels when you create pull requests here, so your PR will be correctly categorized in the release notes.

[shiyu1994]

I think doing the following would prevent adding new complexity to .ci/test.sh:

Thanks for the suggestion. Already applied via 0206da8 and eb34dd5.

We should never merge a PR that will result in CI being broken on master, so please do one of these:

1. push a fix for that issue to this PR
2. remove the cuda_exp CI jobs in this PR, document the work to add them back in a separate issue, merge this PR, and then push a follow-up PR fixing cuda_exp and adding back the CI jobs

I'd prefer the first choice, and already pushed the fixes via ca7df38.

As I mentioned in #5403 (comment) and #5413 (comment), please add a labels when you create pull requests here, so your PR will be correctly categorized in the release notes.

Thanks again for the reminder.

[shiyu1994]

Note that this is blocking other related PRs, including (#5425 and #4827).

[shiyu1994]

This is ready. @jameslamb @guolinke Could you please review this again. If it looks good to you, let's merge this so that we can work on related PRs. Thanks.

[StrikerRUS]

LGTM for .github/workflows/cuda.yml and tests/python_package_test/test_utilities.py except one minor simplification.

[StrikerRUS]

Aren't these checks slow? Maybe move them under #ifdef DEBUG?

[shiyu1994]

Thanks. These checks are fast.

dismissing to prevent blocking, as I might not be able to review again today

[jameslamb]

I don't have any additional comments.

Please address @StrikerRUS 's suggestions, and then I think @guolinke should re-review this (since you've added significant changes since his original review) prior to this being merged.

Thanks for fixing this!

[shiyu1994]

/gha run r-valgrind

Workflow R valgrind tests has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/2945515006

Status: success ✔️.

[shiyu1994]

/gha run r-solaris

[jameslamb]

@shiyu1994 Solaris support was removed in #5226. The r-solaris check no longer exists, and is no longer necessary.

But thank you for being so thorough!

[shiyu1994]

@guolinke @jameslamb @StrikerRUS Thanks for reviewing this. I'll merge this since all tests have been passed and it is blocking many related PRs including #4827, #5425 and #4266.

[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.