-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feature: Torch dependency in sagameker-core to be made optional (5457) #5713
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
aviruthen
merged 4 commits into
aws:master
from
aviruthen:feature/torch-dependency-in-sagameker-core-to-be-made-5457
Apr 15, 2026
+231
−6
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
89be62e
feature: Torch dependency in sagameker-core to be made optional (5457)
aviruthen 57b5c02
fix: address review comments (iteration #1)
aviruthen 0ed06b2
fix: address review comments (iteration #1)
aviruthen 5280803
fix: use subprocess instead of importlib.reload to avoid breaking six…
aviruthen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
aviruthen marked this conversation as resolved.
aviruthen marked this conversation as resolved.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
188 changes: 188 additions & 0 deletions
188
sagemaker-core/tests/unit/test_optional_torch_dependency.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"). You | ||
| # may not use this file except in compliance with the License. A copy of | ||
| # the License is located at | ||
| # | ||
| # http://aws.amazon.com/apache2.0/ | ||
| # | ||
| # or in the "license" file accompanying this file. This file is | ||
| # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF | ||
| # ANY KIND, either express or implied. See the License for the specific | ||
| # language governing permissions and limitations under the License. | ||
| """Tests to verify torch dependency is optional in sagemaker-core. | ||
|
|
||
| The "module imports without torch" tests use subprocess instead of | ||
| importlib.reload to avoid poisoning the class hierarchy in the current | ||
| process. six.with_metaclass + old-style super() breaks when a module | ||
| is reloaded because the class identity changes, causing | ||
| ``TypeError: super(type, obj): obj must be an instance or subtype of type`` | ||
| in subsequent tests that instantiate serializers/deserializers. | ||
| """ | ||
| from __future__ import absolute_import | ||
|
|
||
| import io | ||
| import subprocess | ||
| import sys | ||
| import textwrap | ||
|
|
||
| import numpy as np | ||
| import pytest | ||
|
|
||
|
|
||
| def _block_torch(): | ||
| """Block torch imports by setting sys.modules['torch'] to None. | ||
|
|
||
| Returns a dict of saved torch submodule entries so they can be restored. | ||
| """ | ||
| torch_keys = [key for key in sys.modules if key.startswith("torch.")] | ||
|
aviruthen marked this conversation as resolved.
|
||
| saved = {key: sys.modules.pop(key) for key in torch_keys} | ||
| saved["torch"] = sys.modules.get("torch") | ||
| sys.modules["torch"] = None | ||
| return saved | ||
|
|
||
|
|
||
| def _restore_torch(saved): | ||
| """Restore torch modules from saved dict.""" | ||
| original_torch = saved.pop("torch", None) | ||
| if original_torch is not None: | ||
| sys.modules["torch"] = original_torch | ||
| elif "torch" in sys.modules: | ||
| del sys.modules["torch"] | ||
| for key, val in saved.items(): | ||
| sys.modules[key] = val | ||
|
|
||
|
|
||
| def test_serializer_module_imports_without_torch(): | ||
| """Verify that non-torch serializers can be imported and instantiated without torch. | ||
|
|
||
| Runs in a subprocess to avoid polluting the current process's class | ||
| hierarchy via importlib.reload (which breaks six.with_metaclass). | ||
| """ | ||
| code = textwrap.dedent("""\ | ||
| import sys | ||
| # Block torch before any sagemaker imports | ||
| sys.modules["torch"] = None | ||
|
|
||
| from sagemaker.core.serializers.base import ( | ||
| CSVSerializer, | ||
| NumpySerializer, | ||
| JSONSerializer, | ||
| IdentitySerializer, | ||
| ) | ||
|
|
||
| assert CSVSerializer() is not None | ||
| assert NumpySerializer() is not None | ||
| assert JSONSerializer() is not None | ||
| assert IdentitySerializer() is not None | ||
| print("OK") | ||
| """) | ||
| result = subprocess.run( | ||
| [sys.executable, "-c", code], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| assert result.returncode == 0, ( | ||
| f"Subprocess failed:\nstdout: {result.stdout}\nstderr: {result.stderr}" | ||
| ) | ||
|
|
||
|
|
||
| def test_deserializer_module_imports_without_torch(): | ||
| """Verify that non-torch deserializers can be imported and instantiated without torch. | ||
|
|
||
| Runs in a subprocess for the same reason as the serializer test above. | ||
| """ | ||
| code = textwrap.dedent("""\ | ||
| import sys | ||
| sys.modules["torch"] = None | ||
|
|
||
| from sagemaker.core.deserializers.base import ( | ||
| StringDeserializer, | ||
| BytesDeserializer, | ||
| CSVDeserializer, | ||
| NumpyDeserializer, | ||
| JSONDeserializer, | ||
| ) | ||
|
|
||
| assert StringDeserializer() is not None | ||
| assert BytesDeserializer() is not None | ||
| assert CSVDeserializer() is not None | ||
| assert NumpyDeserializer() is not None | ||
| assert JSONDeserializer() is not None | ||
| print("OK") | ||
| """) | ||
| result = subprocess.run( | ||
| [sys.executable, "-c", code], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| assert result.returncode == 0, ( | ||
| f"Subprocess failed:\nstdout: {result.stdout}\nstderr: {result.stderr}" | ||
| ) | ||
|
|
||
|
aviruthen marked this conversation as resolved.
|
||
|
|
||
| def test_torch_tensor_serializer_raises_import_error_without_torch(): | ||
| """Verify TorchTensorSerializer raises ImportError when torch is not installed.""" | ||
| import sagemaker.core.serializers.base as ser_module | ||
|
|
||
| saved = {} | ||
| try: | ||
| saved = _block_torch() | ||
|
|
||
| with pytest.raises(ImportError, match="Unable to import torch"): | ||
| ser_module.TorchTensorSerializer() | ||
| finally: | ||
| _restore_torch(saved) | ||
|
|
||
|
|
||
| def test_torch_tensor_deserializer_raises_import_error_without_torch(): | ||
| """Verify TorchTensorDeserializer raises ImportError when torch is not installed.""" | ||
| import sagemaker.core.deserializers.base as deser_module | ||
|
|
||
| saved = {} | ||
| try: | ||
| saved = _block_torch() | ||
|
|
||
| with pytest.raises(ImportError, match="Unable to import torch"): | ||
| deser_module.TorchTensorDeserializer() | ||
| finally: | ||
| _restore_torch(saved) | ||
|
aviruthen marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def test_torch_tensor_serializer_works_with_torch(): | ||
| """Verify TorchTensorSerializer works when torch is available.""" | ||
| try: | ||
| import torch | ||
| except ImportError: | ||
| pytest.skip("torch is not installed") | ||
|
|
||
| from sagemaker.core.serializers.base import TorchTensorSerializer | ||
|
|
||
| serializer = TorchTensorSerializer() | ||
| tensor = torch.tensor([1.0, 2.0, 3.0]) | ||
| result = serializer.serialize(tensor) | ||
| assert result is not None | ||
| # Verify the result can be loaded back as numpy | ||
| array = np.load(io.BytesIO(result)) | ||
| assert np.array_equal(array, np.array([1.0, 2.0, 3.0])) | ||
|
aviruthen marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def test_torch_tensor_deserializer_works_with_torch(): | ||
| """Verify TorchTensorDeserializer works when torch is available.""" | ||
| try: | ||
| import torch | ||
| except ImportError: | ||
| pytest.skip("torch is not installed") | ||
|
|
||
| from sagemaker.core.deserializers.base import TorchTensorDeserializer | ||
|
|
||
| deserializer = TorchTensorDeserializer() | ||
| # Create a numpy array, save it, and deserialize to tensor | ||
| array = np.array([1.0, 2.0, 3.0]) | ||
| buffer = io.BytesIO() | ||
| np.save(buffer, array) | ||
| buffer.seek(0) | ||
|
|
||
| result = deserializer.deserialize(buffer, "tensor/pt") | ||
| assert isinstance(result, torch.Tensor) | ||
| assert torch.equal(result, torch.tensor([1.0, 2.0, 3.0])) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.