Skip to content
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

Move recursive loading of DataContainer to HDF5Content #1391

Merged
merged 7 commits into from
Mar 25, 2024
Merged

Move recursive loading of DataContainer to HDF5Content #1391

merged 7 commits into from
Mar 25, 2024

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Mar 19, 2024

This will allow users and internal code to use job.content[...] if they what they are looking for is saved in HDF5 storage and obviate the need to rely on job[...] which may be slow because it is also searching the compressed job files.

I will need some logic like this to fix the slow loading of pyiron_atomistics jobs.

pmrv added 5 commits March 19, 2024 17:23
This will allow users and internal code to use job.content[...] if they what they are looking for
is saved in HDF5 storage and obviate the need to rely on job[...] which may be slow because it is
also searching the compressed job files.

I will need some logic like this to fix the slow loading of pyiron_atomistics jobs.
old name made no sense
@pmrv pmrv added the integration Start the integration tests with pyiron_atomistics/contrib for this PR label Mar 22, 2024
@pmrv pmrv marked this pull request as ready for review March 22, 2024 15:23
@pmrv
Copy link
Contributor Author

pmrv commented Mar 22, 2024

The last commit should actually fix pyiron/pyiron_atomistics#1348 and pyiron/pyiron_atomistics#1350, but I'll probably add them in anyway for cleanliness.

As of this PR one can access various things in a job like this:

  • job.project_hdf5 access to the raw HDF5 layout
  • job.content access to HDF, but try to load any DataContainer along the given path (this was previously exclusive to job[...]) this could be extended to other objects that can be to_object()ed.
  • job.files access to files
  • job[...] will try to act first like content, then like files if a string without slashes was passed and finally will try to look up child jobs.

Copy link
Member

@jan-janssen jan-janssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Still we should think about adding a benchmark which times typical job[item] queries and make sure we do not merge changes in the future which slow down.

@pmrv pmrv merged commit c1aa430 into main Mar 25, 2024
25 of 27 checks passed
@pmrv pmrv deleted the content branch March 25, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration Start the integration tests with pyiron_atomistics/contrib for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants