-
Notifications
You must be signed in to change notification settings - Fork 195
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
Allow user pluggable serializers and add a demonstration ProxyStore serializer #2842
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
4cb6544
Remove assumption that _identifier attribute exists; compute identifi…
benclifford d5eaf87
ongoing work
benclifford 0c329bb
Move serialization error definition into parsl.serialize
benclifford 9360907
Merge branch 'benc-serializer-exception' into benc-serializer-pluggab…
benclifford e57b6c6
Add a bit of error handling and make deserialize call in only one place.
benclifford e3865e4
fix f-string
benclifford 77123ee
Correct f-string
benclifford fe76699
Add proxystore dependency
benclifford 6926910
add basic proxystore serializer with raw test
benclifford e07cc4b
rename test
benclifford 23da73d
Use unique store names
benclifford 70910e8
Remove spurious debug variable
benclifford 5d827de
Move another serialization exception; fix docs build
benclifford c316957
Fix f-string
benclifford d2efa60
bugfix exception attributes
benclifford 385d39a
check types of serialized values
benclifford 6e99769
Merge remote-tracking branch 'origin/master' into benc-serializer-plu…
benclifford 6c21740
more work to make tests pass
benclifford fe28418
Rearrange imports to only import proxystore when actually executing p…
benclifford 3b16da8
Install proxystore for --config local tests
benclifford f95eceb
fixup docstring
benclifford 4f4ee5a
Merge branch 'master' into benc-serializer-pluggable-identifier
benclifford 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 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 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 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 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 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 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 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,49 @@ | ||
import dill | ||
import io | ||
import typing as t | ||
|
||
from parsl.serialize.base import SerializerBase | ||
from proxystore.store import Store | ||
|
||
|
||
class ProxyStoreDeepPickler(dill.Pickler): | ||
"""This class extends dill so that certain objects will be stored into | ||
ProxyStore rather than serialized directly. The selection of objects is | ||
made by a user-specified policy. | ||
""" | ||
|
||
def __init__(self, *args: t.Any, should_proxy: t.Callable[[t.Any], bool], store: Store, **kwargs: t.Any) -> None: | ||
super().__init__(*args, **kwargs) | ||
self._store = store | ||
self._should_proxy = should_proxy | ||
|
||
def reducer_override(self, o: t.Any) -> t.Any: | ||
if self._should_proxy(o): | ||
proxy = self._store.proxy(o) | ||
return proxy.__reduce__() | ||
else: | ||
# fall through to dill | ||
return NotImplemented | ||
|
||
|
||
class ProxyStoreSerializer(SerializerBase): | ||
|
||
def __init__(self, *, should_proxy: t.Optional[t.Callable[[t.Any], bool]] = None, store: t.Optional[Store] = None) -> None: | ||
self._store = store | ||
self._should_proxy = should_proxy | ||
|
||
def serialize(self, data: t.Any) -> bytes: | ||
assert self._store is not None | ||
assert self._should_proxy is not None | ||
|
||
assert data is not None | ||
|
||
f = io.BytesIO() | ||
pickler = ProxyStoreDeepPickler(file=f, store=self._store, should_proxy=self._should_proxy) | ||
pickler.dump(data) | ||
return f.getvalue() | ||
|
||
def deserialize(self, body: bytes) -> t.Any: | ||
# because we aren't customising deserialization, use regular | ||
# dill for deserialization | ||
return dill.loads(body) |
82 changes: 82 additions & 0 deletions
82
parsl/tests/test_serialization/test_proxystore_configured.py
This file contains 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,82 @@ | ||
import logging | ||
import pytest | ||
import uuid | ||
|
||
import parsl | ||
from parsl.serialize.facade import additional_methods_for_deserialization, methods_for_data, register_method_for_data | ||
from parsl.tests.configs.htex_local import fresh_config | ||
|
||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def local_setup(): | ||
from parsl.serialize.proxystore import ProxyStoreSerializer | ||
from proxystore.store import Store, register_store | ||
from proxystore.connectors.file import FileConnector | ||
|
||
parsl.load(fresh_config()) | ||
|
||
store = Store(name='parsl_store_' + str(uuid.uuid4()), connector=FileConnector(store_dir="/tmp")) | ||
register_store(store) | ||
|
||
s = ProxyStoreSerializer(store=store, should_proxy=policy_example) | ||
|
||
global previous_methods | ||
previous_methods = methods_for_data.copy() | ||
|
||
# get rid of all data serialization methods, in preparation for using only | ||
# proxystore. put all the old methods as additional methods used only for | ||
# deserialization, because those will be needed to deserialize the results, | ||
# which will be serialized using the default serializer set. | ||
additional_methods_for_deserialization.update(previous_methods) | ||
methods_for_data.clear() | ||
|
||
register_method_for_data(s) | ||
logger.info(f"BENC: methods for data: {methods_for_data}") | ||
|
||
|
||
def local_teardown(): | ||
parsl.dfk().cleanup() | ||
parsl.clear() | ||
|
||
methods_for_data.clear() | ||
methods_for_data.update(previous_methods) | ||
|
||
additional_methods_for_deserialization.clear() | ||
|
||
|
||
@parsl.python_app | ||
def identity(o): | ||
return o | ||
|
||
|
||
@parsl.python_app | ||
def is_proxy(o) -> bool: | ||
from proxystore.proxy import Proxy | ||
return isinstance(o, Proxy) | ||
|
||
|
||
def policy_example(o): | ||
"""Example policy will proxy only lists.""" | ||
return isinstance(o, frozenset) | ||
|
||
|
||
@pytest.mark.local | ||
def test_proxystore_via_apps(): | ||
from proxystore.proxy import Proxy | ||
|
||
# check roundtrip for an int, which should not be proxystored according | ||
# to example_policy() | ||
roundtripped_7 = identity(7).result() | ||
assert roundtripped_7 == 7 | ||
assert not isinstance(roundtripped_7, Proxy) | ||
|
||
# check roundtrip for a list, which should be proxystored according | ||
# to example_policy() | ||
v = frozenset([1, 2, 3]) | ||
|
||
assert is_proxy(v).result() | ||
|
||
roundtripped_v = identity(v).result() | ||
assert roundtripped_v == v |
This file contains 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,38 @@ | ||
import pytest | ||
import uuid | ||
|
||
|
||
def policy_example(o): | ||
"""Example policy will proxy only lists.""" | ||
return isinstance(o, list) | ||
|
||
|
||
@pytest.mark.local | ||
def test_proxystore_nonglobal(): | ||
"""Check that values are roundtripped, for both proxied and non-proxied types. | ||
""" | ||
# import in function, because proxystore is not importable in base parsl | ||
# installation. | ||
from parsl.serialize.proxystore import ProxyStoreSerializer | ||
from proxystore.proxy import Proxy | ||
from proxystore.store import Store, register_store | ||
from proxystore.connectors.file import FileConnector | ||
|
||
store = Store(name='parsl_store_' + str(uuid.uuid4()), connector=FileConnector(store_dir="/tmp")) | ||
register_store(store) | ||
|
||
s = ProxyStoreSerializer(store=store, should_proxy=policy_example) | ||
|
||
# check roundtrip for an int, which will not be proxystored | ||
s_7 = s.serialize(7) | ||
assert isinstance(s_7, bytes) | ||
roundtripped_7 = s.deserialize(s_7) | ||
assert roundtripped_7 == 7 | ||
assert not isinstance(roundtripped_7, Proxy) | ||
|
||
v = [1, 2, 3] | ||
s_v = s.serialize(v) | ||
assert isinstance(s_7, bytes) | ||
roundtripped_v = s.deserialize(s_v) | ||
assert roundtripped_v == v | ||
assert isinstance(roundtripped_v, Proxy) |
This file contains 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be double-check,
__name__
and not__qualname__
is intended?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the cases where
__qualname__
and__name__
differ, this is often going to break: the module + name needs to be something importable.One case that could work, a class defined directly inside another class without any enclosing function/closure, I would have to poke at the import side of things to see how that would work.