-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Auto Scheduler] Upgrade autoscheduler xgboost callback #12144
[Auto Scheduler] Upgrade autoscheduler xgboost callback #12144
Conversation
In pr-head CI check, an error happens:
I notice xgboost is not imported at beginning of this file. How could I use xgboost.callback.TrainingCallBack to define callback function? Any advices here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sending this PR! Would that be possible that we can add some specific tests? Also it would be great if this change can be compatible with older versions of xgboost
just in case.
88ce5be
to
8edc397
Compare
Please review this PR again @shingjan |
The review has been stale for 4 days without any update. To unblock the progress, I would suggest that someone (maybe @Hzfengsy) to validate it locally and then get it merged at our earliest convenience |
I am still wait for #12141 to be reviewed so that I can follow its design(It seems that that pr have been done, I will finish unit test soon). But It's fine this pr can be merged now. |
I don't think a unit test is viable specifically for xgboost version compatibility because (IIUC) our CI instance could only install one version of xgboost... |
Hi, sorry I am quarantined because COVID-19 and this work was put on hold. Can we merge this PR now? Or unit test is necessary? @junrushao |
CC @zxybazh would you like to review this PR? Thanks a lot! |
Hi @Sunny-Island, I wonder if you are still on this PR? There are quite some changes introduced to the use of xgboost in meta schedule after your PR. I wonder if you are willing to incorporate those into your PR and I can take another look after that. Otherwise I can take this over and make the necessary change to upgrade xgboost. Let me know if that works. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- This works fine with latest xgboost 1.7.2 @ 2022-12-09
- Tested on tune_network_x86 demonstrator using autoscheduler
$ rpm -q xgboost-python3
xgboost-python3-1.7.2-20221209.0.git42e6fbb0.cu11_8.fc38.x86_64
$ rpm -q tvm tvm-python3
tvm-0.11-20221209.0.git0dc26dd8.cu11_8.fc38.x86_64
tvm-python3-0.11-20221209.0.git0dc26dd8.cu11_8.fc38.x86_64
Cc @driazati for help to see if this PR pass builders having a bumped xgboost >= 1.7.2
I'm assuming this has been superceded by #14036 |
Fixes #12009
This pull request is related to issue [Bug] xgboost version conflict #12009 .
I test the new code in CentOS system and xgboost==1.6.0 with a llvm-based auto_schedule task. The task works without any regression problem.
I will work on meta_schedule later, and pay attention to any reviews in this PR.