Serialization refactor#97
Merged
Merged
Conversation
Some analysis and comments to remember the past
currently, Callables and partial bork serialisation (these come from _create_apps calls). In general, I think we should not use this construct, and simply have the apps as class methods, avoiding the problem.
replace _create_apps methods and instead create apps when required: - get_app for Hamiltonian - get_train_app and get_initialise_app for Models This simplifies serialisation (no more Callable instance attributes) and makes the apps react to changes (e.g., changing the force constant in EinsteinCrystal or the parsl_resource_specification in MaceHamiltonian) and should really not cause any noticeable overhead.
- get_execute_app for Reference instances - also deconvolute some logic API unchanged
pdobbelaere
force-pushed
the
serialization
branch
from
8566e38 to
59b89a8
Compare
pdobbelaere
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
(only merge after #96)
fixes #78
Reimplementation of the
psiflow.serializationmodule.Before this PR, serialization was based on class introspection of type hints to determine how a psiflow class instance should be serialized. Since the Python typing system evolves continuously, this locked psiflow into v3.10. More fundamentally, type hints should not control runtime decisions.
New functionality:
resolve_nested_futuresinpsiflow.utils.futureto resolve futures hidden in complex data trees (here)Updated functionality (
psiflow.serialization):serializeis reimplemented, but does the samedeserializeis now an app, so works with futures. The regular function is renamed to_deserializeDeveloper notes:
psiflow.register_serializable_create_appsfunctions inReference,HamiltonianandModel. Functionality unchanged.Referencelogic. Functionality unchanged.After this PR, we can almost update to the latest Python version. The new bottleneck is MACE and PyTorch.