Metrics changes #286
Metrics changes #286
Conversation
|
Build is failing with an error: Please replace these lines with following: |
docker/1.5-1/final/Dockerfile.cpu
Outdated
| @@ -1,4 +1,4 @@ | |||
| ARG SAGEMAKER_XGBOOST_VERSION=1.5-1 | |||
| iARG SAGEMAKER_XGBOOST_VERSION=1.5-1 | |||
There was a problem hiding this comment.
Oh oops, sorry about that!
docker/1.5-1/final/Dockerfile.cpu
Outdated
| @@ -15,7 +15,9 @@ RUN python3 -m pip install -r /requirements.txt && rm /requirements.txt | |||
| # Copy wheel to container # | |||
| ########################### | |||
| COPY dist/sagemaker_xgboost_container-2.0-py2.py3-none-any.whl /sagemaker_xgboost_container-1.0-py2.py3-none-any.whl | |||
| RUN rm -rf /miniconda3/lib/python3.7/site-packages/numpy-1.21.2.dist-info && \ | |||
|
|
|||
| RUN conda install numpy --force-reinstall && \ | |||
There was a problem hiding this comment.
Why do we force-reinstall the numpy here? Will it install the latest numpy version? Shouldn't we pin https://github.com/aws/sagemaker-xgboost-container/blob/master/requirements.txt#L11 here and use pinned numpy?
There was a problem hiding this comment.
The force-reinstall was due to an earlier comment. I did try running it without and there is a build error associated with numpy. I believe the reinstall will reinstall the current version and not the latest version but I will confirm this is the case when the check finishes.
There was a problem hiding this comment.
Can you please try to update numpy version to numpy==1.21.6 in requirements.txt and check if this change can be avoided?
There was a problem hiding this comment.
The change to the numpy version has been added!
|
Integration Tests for the new metrics (MAE and RMSE): [mxnet-algorithms-integ-test-220601-191722-724128] |
eugskim
force-pushed
the
MetricsChanges
branch
2 times, most recently
from
5abd69d
to
2552292
Compare
Co-authored-by: Mark Bunday <mabunday@amazon.com>
* Bump docker runtime version from 17 to 19 * Remove explicit requests install from buildspec Co-authored-by: Mark Bunday <mabunday@amazon.com>
|
Can you please confirm if the integ tests were run for an image containing these changes? |
Yes, the integration testing was ran on a docker image with the new changes in this PR |
Issue #, if available:
Description of changes:
Adding new metrics and tests
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.