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

XGBoost 2.0.0 breaks tests #732

Open
ksaur opened this issue Sep 18, 2023 · 8 comments · May be fixed by #745
Open

XGBoost 2.0.0 breaks tests #732

ksaur opened this issue Sep 18, 2023 · 8 comments · May be fixed by #745
Labels
help wanted Extra attention is needed

Comments

@ksaur
Copy link
Contributor

ksaur commented Sep 18, 2023

XGB 2.0.0 was released last week (Sept 12, 2023). This causes 19 tests to fail.

Many errors are related to NDCG ("NDCG is now the default objective function.")

@ksaur
Copy link
Contributor Author

ksaur commented Sep 18, 2023

Before, I believe they were defaulting to objective="rank:pairwise" but that doesn't seem to fix this.
Also, we'll need an upstream fix from onnxmltools for the 1 onnx XBG error.

Started troubleshooting at #733

Pinning this for now.

@ksaur
Copy link
Contributor Author

ksaur commented Nov 17, 2023

onnxmltools was updated but let's wait for their release, then do ours

@mshr-h mshr-h linked a pull request Dec 25, 2023 that will close this issue
@ksaur
Copy link
Contributor Author

ksaur commented Jan 4, 2024

With the onnxmltools patch, we now get some new errors:
==== 19 failed, 516 passed, 105 skipped, 115 warnings in 294.32s (0:04:54) ====

Some of them are related to the ndcg change

FAILED ....Check failed: label_is_valid: Relevance degress 
must be lesser than or equal to 31 when the exponential NDCG gain function is used. 
Set `ndcg_exp_gain` to false to use custom DCG gain.

but many are just outright wrong: Mismatched elements: 100 / 100 (100%) ...

@ksaur
Copy link
Contributor Author

ksaur commented Jan 4, 2024

The problem is related to convert_sklearn_xgb_regressor's

tree_infos = operator.raw_operator.get_booster().get_dump()

@ksaur
Copy link
Contributor Author

ksaur commented Jan 4, 2024

With xgb==1.7.6, for get_dump() we have '0:[f0<0.84472543] yes=1,no=2,missing=1\n\t1:leaf=-0.0600000024\n\t2:leaf=0.100000009\n',

whereas with xgb==2.0.0 we have '0:leaf=2.36637643e-09\n' from get_dump()

Then at:

return TreeParameters(lefts, rights, features, thresholds, values)

the values for TreeParameters(lefts, rights, features, thresholds, values) are incorrect

@ksaur
Copy link
Contributor Author

ksaur commented Jan 5, 2024

Since it seems they likely changed something internally (now it appears there is a one-node tree rather than a three-node tree), this will take a bit more time. Let's not block the release on this for now.

@ksaur
Copy link
Contributor Author

ksaur commented Jan 9, 2024

TODO: look at onnx/onnxmltools#597 more closely

@ksaur
Copy link
Contributor Author

ksaur commented Mar 8, 2024

Going forward we can look at #764 as a starting point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant