Repository navigation
Request to revert py_version to optional in SDK v2 #1810
Description
Describe the feature you'd like
As the release of Python 3.0 fades into distant memory (back in 2008: Between Windows Vista and 7) and the current set of container offerings seem to have taken a pretty consistent approach of supporting exactly one minor version of Python 3 per minor framework version, the timing for SageMaker SDK to make py_version mandatory seems a bit strange?
I didn't really pick up on it when first looking through, but it felt jarring when exploring now that the package is on PyPI: Figuring out whether I'm allowed to just put py3 or need to look up the supported minor version for the container is extra work, and since only one py3 sub-version is typically supported it doesn't seem to offer much benefit in return.
How would this feature be used? Please describe.
Where not specified, SDK would assume the most recent supported version of Python for the given framework version.
Being able to omit the parameter would reduce complexity of getting started and cut out a parameter which is most often over-specified to exactly one appropriate value anyway.
Describe alternatives you've considered
I've not been close to the discussion on why this change was made so maybe there's a good reason I missed:
- Maybe there were some previous configurations that defaulted to
py2for backward compatibility reasons where apy3option was available? Admit that would be something important to get away from, and a straight-to-default-py3 upgrade path might have been tricky - Perhaps this is a deliberate effort to boost user awareness of which minor Python version is running in the container, and emphasise that it may be different from the notebook? Agree that this awareness is important, but would look for other options if possible
- Maybe there are plans to support more comprehensive sets of Python versions for each framework version? That'd be cool, but seems like a lot of added effort?
Additional context
Thanks for all your hard work in preparing v2!
Activity
chuyang-deng commentedon Aug 5, 2020
Hi @athewsey,
Due to the fact that we are releasing multiple py3.x framework containers, and the inconsistency of container py_version, we want to require
py_versionparam in the Python SDK.By requiring
py_versioncustomer won't see any surprises regarding Python versions after they launch a training/serving job (and billing starts). For example, some customers training/inference scripts are in py36 but a framework's latest Python version is py37 (which will be defaulted to per your suggestion), this might cause script error after a job has already started. We think it's better to catch the error earlier than later.athewsey commentedon Oct 1, 2020
Thanks for clarifying on the reasoning @ChuyangDeng... But just to confirm for me this request still stands:
Per the Deep Learning Containers list as of today - only a single Framework-minorversion seems to offer multiple Py3 version options (TensorFlow v1.15.2 and v1.15.3) and even in this case, Py3.7 is not available for inference so only Py3.6 is fully supported.
In all the other listed cases, there seems to be exactly one supported Py3 minor version per framework minor version:
This is also not true as the current implementation stands because, per the docs for SKLearn(), MXNet(), PyTorch(), and XGBoost(), the ambiguous
py3value is still supported and in some cases even required (onlypy3orpy2options).I actually encountered an issue just this week when SKLearn container was upgraded from Py3.7 to Py3.8... And there doesn't seem to be clear documentation on available SKLearn and Spark container versions as there is for DLCs - at least that I could find linked from the SageMaker doc page.
I would still argue to revert this parameter to optional unless:
py3value is disallowed (so the required parameter actually provides the protection it's trying to)