[c++] Fixed Predictor lifecycle and trees initialization in Contrib mode

[AndreyOrb]

1. Fixed Predictor lifecycle
2. Fixed Boosting trees initialization

Fixed #5482

[jameslamb]

Thanks for this! Will defer to other reviewers with more C++ experience but one question... is it possible to test that correctness of this by adding test cases in https://github.com/microsoft/LightGBM/tree/master/tests/cpp_tests?

There are tests on prediction there, but I specifically mean testing that this previously-not-thread-safe behavior remains thread-safe.

That'd reduce the risk of issues be re-introduced in future refactorings.

[AndreyOrb]

@jameslamb, I've tried to. Unfortunately, the tests fail (not my tests, I reverted my changes ensure that it's not my code).
We can try again after the baseline tests are fixed.

Where should I add the test? In test_single_row.cpp ?

[jameslamb]

Ah interesting. Looking at that error, I suspect the issue is the working directory you're running from. That test has hard-coded relative paths from the root of the repo, so the testlightgbm binary only works if invoked from the root of the repo.

Here's how we run the tests in continuous integration:

LightGBM/.ci/test.sh

Lines 61 to 63 in e0c34e7

	cmake -B build -S . "${cmake_args[@]}"
	cmake --build build --target testlightgbm -j4 || exit 1
	./testlightgbm || exit 1

Where should I add the test? In test_single_row.cpp ?

Yep that seems like a good place, thanks!

[AndreyOrb]

You are right. Changing the working directory has helped.

Baseline tests have passed:

I have updated the tests to check Contrib prediction type.
Here I can see that several threads enter the "danger" code area:

The new test before the fix manages to crash the test:

I used 100 iterations to ensure I hit the raise condition:

But then, the test is becoming slow:

For production, I reduced the number to 5:

[jameslamb]

/gha run r-valgrind

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

Status: success ✔️.

[AndreyOrb]

Is there anything else I should do to push the PR?

[jameslamb]

I have no other comments. Hoping @shiyu1994 or @guolinke can review this when they have time.

[AndreyOrb]

@shiyu1994, @guolinke Could you review the PR?
I'm afraid I will not be able to fix the PR later, if anything pops up.

[guolinke]

Thank you!

[StrikerRUS]

LGTM, thanks a lot for your PR!

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