Skip to content

infra: use fixture for Chainer and XGBoost Python version, clean up remaining version fixtures #1631

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

Merged
merged 2 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 28 additions & 37 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,6 @@ def pytest_addoption(parser):
parser.addoption("--sagemaker-client-config", action="store", default=None)
parser.addoption("--sagemaker-runtime-config", action="store", default=None)
parser.addoption("--boto-config", action="store", default=None)
parser.addoption("--chainer-full-version", action="store", default="5.0.0")
parser.addoption("--ei-mxnet-full-version", action="store", default="1.5.1")
parser.addoption(
"--rl-coach-mxnet-full-version",
action="store",
default=RLEstimator.COACH_LATEST_VERSION_MXNET,
)
parser.addoption(
"--rl-coach-tf-full-version", action="store", default=RLEstimator.COACH_LATEST_VERSION_TF
)
parser.addoption(
"--rl-ray-full-version", action="store", default=RLEstimator.RAY_LATEST_VERSION
)
parser.addoption("--ei-tf-full-version", action="store")
parser.addoption("--xgboost-full-version", action="store", default="1.0-1")


def pytest_configure(config):
Expand Down Expand Up @@ -248,8 +233,13 @@ def rl_ray_version(request):


@pytest.fixture(scope="module")
def chainer_full_version(request):
return request.config.getoption("--chainer-full-version")
def chainer_full_version():
Copy link
Contributor

@ajaykarpur ajaykarpur Jun 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope, but are we still supporting Chainer? Should this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's been no public announcement to say we're not supporting it.

return "5.0.0"


@pytest.fixture(scope="module")
def chainer_full_py_version():
return "py3"


@pytest.fixture(scope="module")
Expand All @@ -263,8 +253,8 @@ def mxnet_full_py_version():


@pytest.fixture(scope="module")
def ei_mxnet_full_version(request):
return request.config.getoption("--ei-mxnet-full-version")
def ei_mxnet_full_version():
return "1.5.1"


@pytest.fixture(scope="module")
Expand All @@ -283,18 +273,18 @@ def pytorch_full_ei_version():


@pytest.fixture(scope="module")
def rl_coach_mxnet_full_version(request):
return request.config.getoption("--rl-coach-mxnet-full-version")
def rl_coach_mxnet_full_version():
return RLEstimator.COACH_LATEST_VERSION_MXNET


@pytest.fixture(scope="module")
def rl_coach_tf_full_version(request):
return request.config.getoption("--rl-coach-tf-full-version")
def rl_coach_tf_full_version():
return RLEstimator.COACH_LATEST_VERSION_TF


@pytest.fixture(scope="module")
def rl_ray_full_version(request):
return request.config.getoption("--rl-ray-full-version")
def rl_ray_full_version():
return RLEstimator.RAY_LATEST_VERSION


@pytest.fixture(scope="module")
Expand Down Expand Up @@ -347,13 +337,19 @@ def tf_full_py_version(tf_full_version):
return "py37"


@pytest.fixture(scope="module", params=["1.15.0", "2.0.0"])
def ei_tf_full_version(request):
tf_ei_version = request.config.getoption("--ei-tf-full-version")
if tf_ei_version is None:
return request.param
else:
tf_ei_version
@pytest.fixture(scope="module")
def ei_tf_full_version():
return "2.0.0"


@pytest.fixture(scope="module")
def xgboost_full_version():
return "1.0-1"


@pytest.fixture(scope="module")
def xgboost_full_py_version():
return "py3"


@pytest.fixture(scope="session")
Expand Down Expand Up @@ -409,8 +405,3 @@ def pytest_generate_tests(metafunc):
):
params.append("ml.p2.xlarge")
metafunc.parametrize("instance_type", params, scope="session")


@pytest.fixture(scope="module")
def xgboost_full_version(request):
return request.config.getoption("--xgboost-full-version")
2 changes: 0 additions & 2 deletions tests/integ/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import logging
import os
import sys

import boto3

Expand All @@ -23,7 +22,6 @@
TUNING_DEFAULT_TIMEOUT_MINUTES = 20
TRANSFORM_DEFAULT_TIMEOUT_MINUTES = 20
AUTO_ML_DEFAULT_TIMEMOUT_MINUTES = 60
PYTHON_VERSION = "py{}".format(sys.version_info.major)

# these regions have some p2 and p3 instances, but not enough for continuous testing
HOSTING_NO_P2_REGIONS = [
Expand Down
11 changes: 5 additions & 6 deletions tests/integ/test_airflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
from sagemaker.utils import sagemaker_timestamp
from sagemaker.workflow import airflow as sm_airflow
from sagemaker.xgboost import XGBoost
from tests.integ import datasets, DATA_DIR, PYTHON_VERSION
from tests.integ import datasets, DATA_DIR
from tests.integ.record_set import prepare_record_set_from_local_files
from tests.integ.timeout import timeout

Expand Down Expand Up @@ -404,7 +404,7 @@ def test_rcf_airflow_config_uploads_data_source_to_s3(sagemaker_session, cpu_ins

@pytest.mark.canary_quick
def test_chainer_airflow_config_uploads_data_source_to_s3(
sagemaker_local_session, cpu_instance_type, chainer_full_version
sagemaker_local_session, cpu_instance_type, chainer_full_version, chainer_full_py_version
):
with timeout(seconds=AIRFLOW_CONFIG_TIMEOUT_IN_SECONDS):
script_path = os.path.join(DATA_DIR, "chainer_mnist", "mnist.py")
Expand All @@ -416,7 +416,7 @@ def test_chainer_airflow_config_uploads_data_source_to_s3(
train_instance_count=SINGLE_INSTANCE_COUNT,
train_instance_type="local",
framework_version=chainer_full_version,
py_version=PYTHON_VERSION,
py_version=chainer_full_py_version,
sagemaker_session=sagemaker_local_session,
hyperparameters={"epochs": 1},
use_mpi=True,
Expand Down Expand Up @@ -545,20 +545,19 @@ def test_tf_airflow_config_uploads_data_source_to_s3(


@pytest.mark.canary_quick
@pytest.mark.skipif(PYTHON_VERSION == "py2", reason="XGBoost container does not support Python 2.")
def test_xgboost_airflow_config_uploads_data_source_to_s3(
sagemaker_session, cpu_instance_type, xgboost_full_version
sagemaker_session, cpu_instance_type, xgboost_full_version, xgboost_full_py_version
):
with timeout(seconds=AIRFLOW_CONFIG_TIMEOUT_IN_SECONDS):
xgboost = XGBoost(
entry_point=os.path.join(DATA_DIR, "dummy_script.py"),
framework_version=xgboost_full_version,
py_version=xgboost_full_py_version,
role=ROLE,
sagemaker_session=sagemaker_session,
train_instance_type=cpu_instance_type,
train_instance_count=SINGLE_INSTANCE_COUNT,
base_job_name="XGBoost job",
py_version=PYTHON_VERSION,
)

training_config = _build_airflow_workflow(
Expand Down
47 changes: 32 additions & 15 deletions tests/integ/test_chainer_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,32 @@
from sagemaker.chainer.estimator import Chainer
from sagemaker.chainer.model import ChainerModel
from sagemaker.utils import unique_name_from_base
from tests.integ import DATA_DIR, PYTHON_VERSION, TRAINING_DEFAULT_TIMEOUT_MINUTES
from tests.integ import DATA_DIR, TRAINING_DEFAULT_TIMEOUT_MINUTES
from tests.integ.timeout import timeout, timeout_and_delete_endpoint_by_name


@pytest.fixture(scope="module")
def chainer_local_training_job(sagemaker_local_session, chainer_full_version):
return _run_mnist_training_job(sagemaker_local_session, "local", 1, chainer_full_version)
def chainer_local_training_job(
sagemaker_local_session, chainer_full_version, chainer_full_py_version
):
return _run_mnist_training_job(
sagemaker_local_session, "local", 1, chainer_full_version, chainer_full_py_version
)


@pytest.mark.local_mode
def test_distributed_cpu_training(sagemaker_local_session, chainer_full_version):
_run_mnist_training_job(sagemaker_local_session, "local", 2, chainer_full_version)
def test_distributed_cpu_training(
sagemaker_local_session, chainer_full_version, chainer_full_py_version
):
_run_mnist_training_job(
sagemaker_local_session, "local", 2, chainer_full_version, chainer_full_py_version
)


@pytest.mark.local_mode
def test_training_with_additional_hyperparameters(sagemaker_local_session, chainer_full_version):
def test_training_with_additional_hyperparameters(
sagemaker_local_session, chainer_full_version, chainer_full_py_version
):
script_path = os.path.join(DATA_DIR, "chainer_mnist", "mnist.py")
data_path = os.path.join(DATA_DIR, "chainer_mnist")

Expand All @@ -45,7 +55,7 @@ def test_training_with_additional_hyperparameters(sagemaker_local_session, chain
train_instance_count=1,
train_instance_type="local",
framework_version=chainer_full_version,
py_version=PYTHON_VERSION,
py_version=chainer_full_py_version,
sagemaker_session=sagemaker_local_session,
hyperparameters={"epochs": 1},
use_mpi=True,
Expand All @@ -62,7 +72,9 @@ def test_training_with_additional_hyperparameters(sagemaker_local_session, chain

@pytest.mark.canary_quick
@pytest.mark.regional_testing
def test_attach_deploy(sagemaker_session, chainer_full_version, cpu_instance_type):
def test_attach_deploy(
sagemaker_session, chainer_full_version, chainer_full_py_version, cpu_instance_type
):
with timeout(minutes=TRAINING_DEFAULT_TIMEOUT_MINUTES):
script_path = os.path.join(DATA_DIR, "chainer_mnist", "mnist.py")
data_path = os.path.join(DATA_DIR, "chainer_mnist")
Expand All @@ -71,7 +83,7 @@ def test_attach_deploy(sagemaker_session, chainer_full_version, cpu_instance_typ
entry_point=script_path,
role="SageMakerRole",
framework_version=chainer_full_version,
py_version=PYTHON_VERSION,
py_version=chainer_full_py_version,
train_instance_count=1,
train_instance_type=cpu_instance_type,
sagemaker_session=sagemaker_session,
Expand Down Expand Up @@ -100,7 +112,12 @@ def test_attach_deploy(sagemaker_session, chainer_full_version, cpu_instance_typ


@pytest.mark.local_mode
def test_deploy_model(chainer_local_training_job, sagemaker_local_session, chainer_full_version):
def test_deploy_model(
chainer_local_training_job,
sagemaker_local_session,
chainer_full_version,
chainer_full_py_version,
):
script_path = os.path.join(DATA_DIR, "chainer_mnist", "mnist.py")

model = ChainerModel(
Expand All @@ -109,7 +126,7 @@ def test_deploy_model(chainer_local_training_job, sagemaker_local_session, chain
entry_point=script_path,
sagemaker_session=sagemaker_local_session,
framework_version=chainer_full_version,
py_version=PYTHON_VERSION,
py_version=chainer_full_py_version,
)

predictor = model.deploy(1, "local")
Expand All @@ -120,7 +137,7 @@ def test_deploy_model(chainer_local_training_job, sagemaker_local_session, chain


def _run_mnist_training_job(
sagemaker_session, instance_type, instance_count, chainer_full_version, wait=True
sagemaker_session, instance_type, instance_count, chainer_version, py_version
):
script_path = (
os.path.join(DATA_DIR, "chainer_mnist", "mnist.py")
Expand All @@ -133,8 +150,8 @@ def _run_mnist_training_job(
chainer = Chainer(
entry_point=script_path,
role="SageMakerRole",
framework_version=chainer_full_version,
py_version=PYTHON_VERSION,
framework_version=chainer_version,
py_version=py_version,
train_instance_count=instance_count,
train_instance_type=instance_type,
sagemaker_session=sagemaker_session,
Expand All @@ -147,7 +164,7 @@ def _run_mnist_training_job(
test_input = "file://" + os.path.join(data_path, "test")

job_name = unique_name_from_base("test-chainer-training")
chainer.fit({"train": train_input, "test": test_input}, wait=wait, job_name=job_name)
chainer.fit({"train": train_input, "test": test_input}, job_name=job_name)
return chainer


Expand Down
5 changes: 1 addition & 4 deletions tests/integ/test_rl.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@

from sagemaker.rl import RLEstimator, RLFramework, RLToolkit
from sagemaker.utils import sagemaker_timestamp, unique_name_from_base
from tests.integ import DATA_DIR, PYTHON_VERSION
from tests.integ import DATA_DIR
from tests.integ.timeout import timeout, timeout_and_delete_endpoint_by_name


@pytest.mark.canary_quick
@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="RL images supports only Python 3.")
def test_coach_mxnet(sagemaker_session, rl_coach_mxnet_full_version, cpu_instance_type):
estimator = _test_coach(
sagemaker_session, RLFramework.MXNET, rl_coach_mxnet_full_version, cpu_instance_type
Expand Down Expand Up @@ -52,7 +51,6 @@ def test_coach_mxnet(sagemaker_session, rl_coach_mxnet_full_version, cpu_instanc
assert 0 < action[0][1] < 1


@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="RL images supports only Python 3.")
def test_coach_tf(sagemaker_session, rl_coach_tf_full_version, cpu_instance_type):
estimator = _test_coach(
sagemaker_session, RLFramework.TENSORFLOW, rl_coach_tf_full_version, cpu_instance_type
Expand Down Expand Up @@ -98,7 +96,6 @@ def _test_coach(sagemaker_session, rl_framework, rl_coach_version, cpu_instance_


@pytest.mark.canary_quick
@pytest.mark.skipif(PYTHON_VERSION != "py3", reason="RL images supports only Python 3.")
def test_ray_tf(sagemaker_session, rl_ray_full_version, cpu_instance_type):
source_dir = os.path.join(DATA_DIR, "ray_cartpole")
cartpole = "train_ray.py"
Expand Down
7 changes: 4 additions & 3 deletions tests/integ/test_tuner.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
datasets,
vpc_test_utils,
DATA_DIR,
PYTHON_VERSION,
TUNING_DEFAULT_TIMEOUT_MINUTES,
)
from tests.integ.record_set import prepare_record_set_from_local_files
Expand Down Expand Up @@ -687,7 +686,9 @@ def test_tuning_tf_vpc_multi(


@pytest.mark.canary_quick
def test_tuning_chainer(sagemaker_session, chainer_full_version, cpu_instance_type):
def test_tuning_chainer(
sagemaker_session, chainer_full_version, chainer_full_py_version, cpu_instance_type
):
with timeout(minutes=TUNING_DEFAULT_TIMEOUT_MINUTES):
script_path = os.path.join(DATA_DIR, "chainer_mnist", "mnist.py")
data_path = os.path.join(DATA_DIR, "chainer_mnist")
Expand All @@ -696,7 +697,7 @@ def test_tuning_chainer(sagemaker_session, chainer_full_version, cpu_instance_ty
entry_point=script_path,
role="SageMakerRole",
framework_version=chainer_full_version,
py_version=PYTHON_VERSION,
py_version=chainer_full_py_version,
train_instance_count=1,
train_instance_type=cpu_instance_type,
sagemaker_session=sagemaker_session,
Expand Down