documentation: add documentation for XGBoost #1350

eslesar-aws · 2020-03-11T22:30:16Z

Description of changes:

Added rst files to generate XGBoost API reference content and explain usage of open source XGBoost in SageMaker.

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I used the commit message format described in CONTRIBUTING
I have used the regional endpoint when creating S3 and/or STS clients (if appropriate)
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…into xgboost-fw

sagemaker-bot · 2020-03-11T22:34:10Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-11T22:53:08Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-11T22:54:39Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-11T23:02:59Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-11T23:12:07Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-11T23:19:38Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-11T23:20:47Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

doc/using_xgboost.rst

laurenyu · 2020-03-12T21:41:13Z

doc/using_xgboost.rst

+  which enables it to scale out to more instances and reduce out-of-memory errors.
+* Exensibility - Because the open source XGBoost container is open source,
+  you can extend the container to install additional libraries and change the version of XGBoost that the container uses.
+  For more information, see `SageMaker XGBoost Container <https://github.com/aws/sagemaker-xgboost-container>`__.


is there a notebook that shows extending the image? if so, I think that would be a better link because the README of the repo launches into how to build and test the image, which isn't particularly relevant for people who just want to use it as a base image in their Dockerfile

I can't find any such notebook. I get that this link isn't ideal, but do you think nothing would be better?

is there documentation for finding the image URI? that's probably the most relevant thing for customers looking to extend the image.

my hesitancy with linking to the instructions for building the image from scratch is that we've had previous GitHub issues where people thought they needed to build the image because that's the README they found, and they hadn't realized that they could just use the pre-built version. to be fair, that might just mean we need to overhaul the framework repository READMEs...

I can't find anything other than the use of the get_image_uri function itself in code examples.

I changed the link to point to the example notebook that extends the pytorch container (https://github.com/awslabs/amazon-sagemaker-examples/blob/master/advanced_functionality/pytorch_extending_our_containers/pytorch_extending_our_containers.ipynb). That is the only example that I can find on extending containers.

doc/using_xgboost.rst

laurenyu · 2020-03-12T21:48:57Z

doc/using_xgboost.rst

+    model_location = args.model_dir + '/xgboost-model'
+    pkl.dump(bst, open(model_location, 'wb'))
+    logging.info("Stored trained model at {}".format(model_location))


might be worth calling out separately that the script needs to save the model and where it has to be saved

Meant to ask about that in email. So is the extra /xgboost-model subdir within model_dir necessary here? Or can it be saved anywhere within model_dir?

bumped on the email thread - I honestly don't know in this case

The intro section says you have to save the model to model_dir, and I added a comment to the part of the script where it saves the model (and an add_argument line for SM_MODEL_DIR). I'll add a section for save model when I get more specific information.

doc/xgboost.rst

…into xgboost-fw

sagemaker-bot · 2020-03-16T22:35:24Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-16T22:55:34Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-16T22:59:25Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-16T23:03:28Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

doc/using_xgboost.rst

laurenyu · 2020-03-17T15:44:38Z

doc/using_xgboost.rst

+
+The XGBoost open source algorithm provides the following benefits over the built-in algorithm:
+
+* Latest version - The open source XGBoost algorithm supports XGBoost version 1.0, which has better performance scaling on multi-core instances and


might be better to remove the version here and just link to https://github.com/aws/sagemaker-python-sdk/tree/master/src/sagemaker/xgboost#xgboost-sagemaker-estimators-and-models, so that fewer places have to be updated with each version upgrade.

although it seems like that page has already gone out of date...

laurenyu · 2020-03-17T15:53:01Z

doc/using_xgboost.rst

+  which enables it to scale out to more instances and reduce out-of-memory errors.
+* Exensibility - Because the open source XGBoost container is open source,
+  you can extend the container to install additional libraries and change the version of XGBoost that the container uses.
+  For more information, see `SageMaker XGBoost Container <https://github.com/aws/sagemaker-xgboost-container>`__.


is there documentation for finding the image URI? that's probably the most relevant thing for customers looking to extend the image.

my hesitancy with linking to the instructions for building the image from scratch is that we've had previous GitHub issues where people thought they needed to build the image because that's the README they found, and they hadn't realized that they could just use the pre-built version. to be fair, that might just mean we need to overhaul the framework repository READMEs...

laurenyu · 2020-03-17T15:55:18Z

doc/using_xgboost.rst

+    # Hyperparameters are described here
+    parser.add_argument('--num_round', type=int)
+    parser.add_argument('--max_depth', type=int, default=5)
+    parser.add_argument('--eta', type=float, default=0.2)
+    parser.add_argument('--objective', type=str, default='reg:squarederror')
+
+    # SageMaker specific arguments. Defaults are set in the environment variables.
+    parser.add_argument('--train', type=str, default=os.environ['SM_CHANNEL_TRAIN'])
+    parser.add_argument('--validation', type=str, default=os.environ['SM_CHANNEL_VALIDATION'])
+
+    args = parser.parse_args()
+
+    train_hp = {
+        'max_depth': args.max_depth,
+        'eta': args.eta,
+        'gamma': args.gamma,
+        'min_child_weight': args.min_child_weight,
+        'subsample': args.subsample,
+        'silent': args.silent,
+        'objective': args.objective
+    }
+
+    dtrain = xgb.DMatrix(args.train)
+    dval = xgb.DMatrix(args.validation)
+    watchlist = [(dtrain, 'train'), (dval, 'validation')] if dval is not None else [(dtrain, 'train')]
+
+    callbacks = []
+    prev_checkpoint, n_iterations_prev_run = add_checkpointing(callbacks)
+    # If checkpoint is found then we reduce num_boost_round by previously run number of iterations
+
+    bst = xgb.train(
+        params=train_hp,
+        dtrain=dtrain,
+        evals=watchlist,
+        num_boost_round=(args.num_round - n_iterations_prev_run),
+        xgb_model=prev_checkpoint,
+        callbacks=callbacks
+    )
+
+    model_location = args.model_dir + '/xgboost-model'
+    pkl.dump(bst, open(model_location, 'wb'))
+    logging.info("Stored trained model at {}".format(model_location))


all of this should be indented to match l. 95

laurenyu · 2020-03-17T15:55:41Z

doc/using_xgboost.rst

+    model_location = args.model_dir + '/xgboost-model'
+    pkl.dump(bst, open(model_location, 'wb'))
+    logging.info("Stored trained model at {}".format(model_location))


bumped on the email thread - I honestly don't know in this case

laurenyu · 2020-03-17T15:56:43Z

doc/using_xgboost.rst

+
+Create an Estimator
+-------------------
+After you create your training script, create an instance of the :class:`sagemaker.xgboost.XGBoost` estimator.


I don't think :class:sagemaker.xgboost.XGBoost` links to anything automatically here

laurenyu · 2020-03-17T15:57:08Z

doc/using_xgboost.rst

+
+.. code::
+
+    from sagemaker.session import s3_input


you can delete the s3_input import

laurenyu · 2020-03-17T15:57:36Z

doc/using_xgboost.rst

+    from sagemaker.session import s3_input
+    from sagemaker.xgboost.estimator import XGBoost
+
+    xgb_script_mode_estimator = XGBoost(


I'd just call the variable xgb or xgb_estimator

laurenyu · 2020-03-17T15:58:04Z

doc/using_xgboost.rst

+Customize inference
+-------------------
+
+In the script that you provide, you can customize the inference behavior by implementing the follwing functions:


follwing --> following

laurenyu · 2020-03-17T15:58:47Z

doc/using_xgboost.rst

+
+In the script that you provide, you can customize the inference behavior by implementing the follwing functions:
+* ``input_fn`` - how input data is handled.
+* ``predict_fn`` - how the model is invokedfunction, and how the response is returned ).


messed up copy/paste?

…into xgboost-fw

sagemaker-bot · 2020-03-27T22:39:39Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-27T22:59:00Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-27T23:07:47Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-27T23:11:18Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-03-30T17:11:28Z

error from the Sphinx check:

Warning, treated as error:
/codebuild/output/src225721869/src/github.com/aws/sagemaker-python-sdk/.tox/sphinx/lib/python3.6/site-packages/sagemaker/xgboost/estimator.py:docstring of sagemaker.xgboost.estimator.XGBoost:11:Unexpected indentation.

eslesar-aws · 2020-03-30T19:01:47Z

I don't see in the xgboost estimator.py file where this indentation is. I didn't change this file.

laurenyu · 2020-03-30T23:05:55Z

I think it's because you added doc/xgboost.rst, which includes the estimator docstrings. My guess is it's these two lines: https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/xgboost/estimator.py#L70-L71

…into xgboost-fw

sagemaker-bot · 2020-03-30T23:42:34Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-31T00:01:23Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-31T00:05:53Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-03-31T00:10:16Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-03-31T15:45:16Z

Warning, treated as error:
/codebuild/output/src121143675/src/github.com/aws/sagemaker-python-sdk/.tox/sphinx/lib/python3.6/site-packages/sagemaker/xgboost/model.py:docstring of sagemaker.xgboost.model.XGBoostModel.prepare_container_def:9:Unexpected indentation.

I think this is https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/xgboost/model.py#L112. Also not entirely sure about the indentation at https://github.com/aws/sagemaker-python-sdk/blob/master/src/sagemaker/xgboost/model.py#L119

…into xgboost-fw

sagemaker-bot · 2020-04-03T00:12:27Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-04-03T00:34:17Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-04-03T00:42:59Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-04-03T00:44:28Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-04-03T16:26:24Z

src/sagemaker/xgboost/model.py

@@ -108,16 +108,15 @@ def __init__(
        self.model_server_workers = model_server_workers

    def prepare_container_def(self, instance_type, accelerator_type=None):
-        """Return a container definition with framework configuration set in model environment
-            variables.
+        """Return a container definition with framework configuration set in model environment variables.


from the failed build:

�[7;33m************* Module sagemaker.xgboost.model�[0m src/sagemaker/xgboost/model.py:111:0: C0301: �[1mLine too long (105/100)�[0m (�[1mline-too-long�[0m)

can you split this line up? (and the second line should be indented at the same level as """)

sagemaker-bot · 2020-04-03T17:40:05Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-04-03T17:56:34Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-04-03T17:59:32Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-04-03T18:04:32Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eslesar-aws added 2 commits March 11, 2020 15:25

documentation: add documentation for XGBoost

96393d5

Merge branch 'master' of https://github.com/aws/sagemaker-python-sdk …

7f2b7e1

…into xgboost-fw

fix: remove blank line in model.py

d7867d5

laurenyu reviewed Mar 12, 2020

View reviewed changes

doc/xgboost.rst Outdated Show resolved Hide resolved

doc/xgboost.rst Outdated Show resolved Hide resolved

eslesar-aws added 2 commits March 16, 2020 10:43

Merge branch 'master' of https://github.com/aws/sagemaker-python-sdk …

b22e7bc

…into xgboost-fw

documentation: made fixes per feedback

24b6166

laurenyu reviewed Mar 17, 2020

View reviewed changes

eslesar-aws added 2 commits March 27, 2020 15:33

documentation: address review comments for XGBoost docs

c71d7e6

Merge branch 'master' of https://github.com/aws/sagemaker-python-sdk …

ec8dbe7

…into xgboost-fw

laurenyu previously approved these changes Mar 30, 2020

View reviewed changes

documentation: fix indentation error in /xgboost/estimator.py docstring

6ea4c8b

Merge branch 'master' of https://github.com/aws/sagemaker-python-sdk …

3b3d964

…into xgboost-fw

eslesar-aws dismissed laurenyu’s stale review via 3b3d964 March 30, 2020 23:37

eslesar-aws added 2 commits April 2, 2020 17:07

documentation: fix indentation error in /xgboost/model.py docstring

ea738dd

Merge branch 'master' of https://github.com/aws/sagemaker-python-sdk …

e025b67

…into xgboost-fw

laurenyu reviewed Apr 3, 2020

View reviewed changes

documentation: fixed long docstring line in /xgboost/model.py

f1c3a1a

laurenyu approved these changes Apr 3, 2020

View reviewed changes

laurenyu merged commit b077737 into aws:master Apr 3, 2020


		The XGBoost open source algorithm provides the following benefits over the built-in algorithm:

		* Latest version - The open source XGBoost algorithm supports XGBoost version 1.0, which has better performance scaling on multi-core instances and

documentation: add documentation for XGBoost #1350

documentation: add documentation for XGBoost #1350

Uh oh!

Conversation

eslesar-aws commented Mar 11, 2020 • edited by laurenyu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 11, 2020

AWS CodeBuild CI Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sagemaker-bot commented Mar 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 16, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Mar 16, 2020

AWS CodeBuild CI Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eslesar-aws commented Mar 11, 2020 •

edited by laurenyu

Loading