Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jvm-packages] clean up example #10618

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Conversation

wbo4958
Copy link
Contributor

@wbo4958 wbo4958 commented Jul 22, 2024

This is the 1st PR of #10617

@wbo4958
Copy link
Contributor Author

wbo4958 commented Jul 22, 2024

Hi @trivialfis @hcho3, please help review this PR which is the first small PR of #10617, thx

Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some questions regarding the examples, rest looks good.

@@ -48,6 +49,6 @@ object BoostFromPrediction {
testMat.setBaseMargin(testPred)

System.out.println("result of running from initial prediction")
val booster2 = XGBoost.train(trainMat, params.toMap, 1, watches.toMap, null, null)
XGBoost.train(trainMat, params.toMap, 1, watches.toMap, null, null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this example doing if it's not showing the result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess it's just trying to verify if the training is ok.

@@ -40,7 +40,6 @@ object CrossValidation {
// set additional eval_metrics
val metrics: Array[String] = null

val evalHist: Array[String] =
XGBoost.crossValidation(trainMat, params.toMap, round, nfold, metrics)
XGBoost.crossValidation(trainMat, params.toMap, round, nfold, metrics)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is an example, maybe it's useful to print it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not to refine the tests in this PR. Maybe have the follow up after merging my all small PRs for reworking jvm

@wbo4958 wbo4958 requested review from trivialfis and hcho3 July 22, 2024 23:00
@trivialfis trivialfis merged commit 003b418 into dmlc:master Jul 23, 2024
32 of 33 checks passed
@wbo4958 wbo4958 deleted the test-clean-up branch July 23, 2024 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants