-
Notifications
You must be signed in to change notification settings - Fork 34
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
Compatibility for xgboost 0.90 #90
Conversation
xgboost_ray/compat/tracker.py
Outdated
# flake8: noqa | ||
# Copied from https://github.com/dmlc/xgboost/blob/master/python-package/xgboost/tracker.py |
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.
Do you need to add some licensing notes 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.
Right, this is Apache 2.0 so probably yeah. I'll quickly check how to attribute this correctly
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.
Ok since our own license is also Apache 2.0 we shouldn't run into any problems.
I still added a license header to this file and linked to the correct (hash-identified) file source and license file on the xgboost repo.
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.
(pushed but github seems to need some time to update this PR)
xgboost_ray/compat/__init__.py
Outdated
logging.warning( | ||
f"You are using `xgboost_ray` with a legacy XGBoost version " | ||
f"(version {xgb.__version__}). While we try to support " | ||
f"older XGBoost versions, please note that this library is only " | ||
f"fully tested and supported for XGBoost >= 1.4. Please consider " | ||
f"upgrading your XGBoost version (`pip install -U xgboost`).") |
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.
i wonder if this will get printed on every single Ray worker?
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.
I don't remember seeing that in my runs but I can check again later to make sure.
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.
Turns out you were right - I moved the warning to the predict()
and train()
methods, so they will only be printed once.
cc @mmui