-
Notifications
You must be signed in to change notification settings - Fork 192
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
Hashing, caching and fast-forwarding #652
Conversation
…ckend) that replaces the _dbnode member if a similar node already exists
…None for hash, which obviously should not be checked in the DB
…ditional extra (hash)
* Failing test for two ArrayData with unequal content * (Accidentally) passing test for two ArrayData of different size, with same str representation For the ArrayData, we need to take the actual array into account when creating the hash, not just the shape which is return in get_attrs()
Add more node hashing tests
Close files after reading them to create the hash.
The caching.defaults.use_cache parameter should be used by plugin developers to mark whether a specific .store() call CAN actually use caching. The user decides whether to use it in the end by setting the default to True / False.
@sphuber @giovannipizzi I've gone through all your comments now. Please check the changes and let me know if there's something else to be changed. |
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 am happy with the changes. Thanks again for an amazing job Dominik
My pleasure. Thanks for all your help! |
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.
Hi Dominik, thanks a lot! I'm approving and merging this, so we also have more people testing it and giving feedback for potential issues.
I think that there are two more things to do after this
- Check and fix Attributes of a calculation can be changed! #1109
- discuss if it is a good idea to store the hash in the extra, or should we have a different internal table, or column in DbNode. The reason being that now all tests have to assume that there is an additional extra, which to me is a bit a strange assumption. Another way would be to decide that all extras starting with aiida are not shown by default by get_extra, and these methods have an additional show_internals=True flag, and the set_extra, without flags, complains if one tries to store something starting with aiida (but there is a flag allow_internal=False by default, to allow it). I'm not sure it's a great idea though, maybe it just complicates a lot the logic, and a new column is just the simplest?
Cool, thanks for merging! For the |
…utes This was already done in PR aiidateam#652 that was merged into develop but needs to be done in this branch as well for consistency, which will be merged into the v0.11.1 patch release
Fixes #119.
Edit / Note: The description below is somewhat out of date, as it describes the initial state of the PR. For a current description, either follow the changes proposed / discussed subsequently, or refer to the descriptions in the documentation made in this PR.
The hashing, caching and fast-forwarding project is a work in progress. Since it is nearing completion, I'm creating this PR to start collecting feedback.
Hashing
The
hashing.make_hash
creates a hash from a given Python datastructure. Nested structures, are handled by recursive hashing, meaning that for example a dict is turned intobefore hashing it. AiiDA
Folder
data can also be hashed, by recursively taking the hash of filenames and contents. For floating point numbers, the last 4 bits of the mantissa are truncated before creating the hash. However, this does not apply toArrayData
, where the array is saved as a file on disk and therefore handled by theFolder
hashing.Using the
make_hash
function, theNode
has aget_hash
method. For basic nodes, this method creates a hash fromget_attrs()
, the database folder, and the__version__
of the module where theNode
class is defined. If there are errors,get_hash
returnsNone
.Node
subclasses can define a class-level_hash_ignored_attributes
list, with names of attributes that will not be taken into account when creating the hash. Edit:_updatabale_attributes
are also ignored now.The
WorkCalculation
subclass addsget_inputs()
to the objects that are hashed inget_hash
, and exposes the_hash_ignored_inputs
to ignore certain inputs.Caching
When a
Node
is stored, the hash is saved as an extra. When subsequent nodes are stored, caching can be enabled by settinguse_cache=True
in thestore
method. This means that a node of the same type and hash will be returned if it exists. Otherwise a new node is created.Nodes can determine whether they can be used as cache by implementing
_is_valid_cache
. This is used to avoid using failed calculations as cache.The
verdi rehash
command can be used to re-calculate the hash either for all nodes, or just for a specific class.Fast-forwarding
Fast-forwarding is implemented by using caching for the calculation of a given
Process
. If fast-forwarding is enabled and there is already such a calculation, this finished calculation will be used. The subsequent steps of running the process are then skipped, and it is done immediately.The process decides whether fast-forwarding is used based on the
_fast_forward_enabled
method. There are two option to set this:_fast_forward=True / False
input to the process.cache_config.yml
in the.aiida
folder. A default can be set, and fast-forwarding for specifc calculation / process classes can be enabled or disabled (by class name). An example file would look like this:Since the fast-forwarding is implemented at the level of
Process
, it doesn't work forInlineCalculation
, andJobCalculation
that is not launched viawork.run.run
orwork.run.submit
.To-Do's
EDIT: Updated To-Do's after a discussion with Giovanni.
_updatable_attributes
in the hash, in addition to_hash_ignored_attributes
.WorkCalculation
adds the inputs to the hash, or ifJobCalculation
also does it. If it doesn't, this needs to be fixed. Update: This is implemented in_get_objects_to_hash
of theAbstractCalculation
, meaning that it works for bothWorkCalculation
andJobCalculation
.plum
is compatible with the caching, and change.travis.yml
to use a release version. Update: Plum version "0.7.11" (not merged) is compatible, but the goal is to merge with plum >= 0.9.verdi rehash
should take PKs as variadic arguments. Add a-a/--all
option to rehash all nodes._is_valid_cache
can be used to permanently disable fast-forwarding on a calculation class. Also add some tests using this, e.g. to have a calculation which creates random output. Update: To disable caching on a class level, the_cacheable
attribute was added. This avoids doing a hash query when the class cannot be cached at all. This does not mean that the nodes don't have a hash, though.cached_from
extra, as a stand-in for theLinkType.CACHE
we might want to add if lazy / reference copying is implemented.ignored_folder_content
(inmake_hash
) does. Update: This is exposed to theget_hash()
method, and is used to ignore theraw_input
for theJobCalculation
.For Folder hashing, handle symbolic links separately. The link target should be hashed instead of the file content, withps
as type string / salt._get_config()
configurable through a keyword argument giving the file path.use_cache
andfast_forward
.JobCalculation
instances.has_failed
andhas_finished
methods to AbstractCalculation, implement in InlineCalculation.