Upgrade XgBoost to 1.7.x

[malav-shastri]

Issue #, if available:

• We are upgrading XgBoost version to 1.7.3 (Update :) 1.7.4, with this first revision of PR we are targeting the Unit test failures which have resulted from this upgrade. -- https://quip-amazon.com/sZqYAQpmTGtR/Breaking-features-in-Xgboost152-173
  (Note: According to the version upgrade process and steps this PR will only be merged after successful integration tests and container test and changes done accordingly in the following revisions)

Description of changes:

• A new callback interface was designed for Python package starting from version 1.3.x. Even though the old callback style was deprecated the code traces were not removed until 1.6.x. And hence we on 1P algorithms side are still using old callback style function and code in 1.5.2 specifically in checkpointing.py. Since in 1.7.3 we can’t use deprecated callback style, there’s a need for updating checkpointing.py and test_checkpointing.py according to the new callback style functions.
• According to open source XgBoost training code the callbacks are applied at the end of each iteration. Here in this new implementation I have override and implemented after_training() function of new callback API in our existing set of callbacks. According to the opensource XgBoost code this will be called after the update function which updates the model after an iteration. After the execution of update function, code calls after_iteration function of CallbackContainer class, which in turns invoke the after_iteration function defined in each of these callback function from the list.
• There's still a failure with test_training.py file, but the error stack trace suggests that it's commit from smdebug hook class we already have a ticket with the debugger team to make their code compatible with the latest XgBoost version and we'll be following up on the issue again. Update we have tested all the unit tests and integration tests without debugger code to check this implementation and we aren't experiencing any failures with the unit and integration tests so far. (Update : ) Debugger has fixed the issue and we are not having any further failures/errors

Testing:

• pytest test/unit/
• flake8 tests
• Integration tests

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[amzn-choeric]

Just adding a side note. Please check to see if the override for tracker.py is really necessary at this point. I believe it was added as an ad-hoc fix until the open source library rolled out the changes on their side, but I have not looked into it too deeply. Also, please make sure that the environment flag for disabling container support is added by default in serving_mms.py as oppose to requiring an opt-in.

[amzn-choeric]

Also, you should be able to run integration tests prior to merging.

[malav-shastri]

Also, you should be able to run integration tests prior to merging.

sure thanks, that's the plan. Will not be merging this prior to ensuring the succeeding integration tests.

[dewan-c]

Looks like you're planning on submitting another revision and testing. LGTM otherwise.

[malav-shastri]

Just adding a side note. Please check to see if the override for tracker.py is really necessary at this point. I believe it was added as an ad-hoc fix until the open source library rolled out the changes on their side, but I have not looked into it too deeply. Also, please make sure that the environment flag for disabling container support is added by default in serving_mms.py as oppose to requiring an opt-in.

I think I'll sync with you offline on this, as I have some questions

[malav-shastri]

Updated the description with some more details on new callback API

[mabunday]

Looks good to me, thanks for the detailed description and deep dive into the new callback behavior.

Optionally, feel free to run black -l 120 src/sagemaker_xgboost_container/checkpointing.py and black -l 120 test/unit/test_checkpointing.py, but this can also be saved for another PR.

[NikhilRaverkar]

Is this not supported in 1.7 anymore?

[malav-shastri]

yes as per my understanding its not supported anymore
I am referring to this set of breaking changes https://github.com/dmlc/xgboost/releases/tag/v1.7.0#:~:text=Single%20precision%20histogram%20is%20removed%20due%20to%20its%20lack%20of%20accuracy%20caused%20by%20significant%20floating%20point%20error.%20In%20some%20cases%20the%20error%20can%20be%20difficult%20to%20detect%20due%20to%20log%2Dscale%20operations%2C%20which%20makes%20the%20parameter%20dangerous%20to%20use