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

use znh5md v0.3 #292

Merged
merged 8 commits into from
Jul 16, 2024
Merged

use znh5md v0.3 #292

merged 8 commits into from
Jul 16, 2024

Conversation

PythonFZ
Copy link
Contributor

@PythonFZ PythonFZ commented Jul 9, 2024

No description provided.

@PythonFZ PythonFZ requested a review from M-R-Schaefer July 9, 2024 14:58
Copy link
Contributor

@M-R-Schaefer M-R-Schaefer 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, I appreciate the leaner API.
You mentioned that it is now possible to keep the H5 file open during the simulation, eliminating some issues we were having. Does the current implementation already make use of this? if not I can work it out in a different branch.

@PythonFZ
Copy link
Contributor Author

PythonFZ commented Jul 9, 2024

looks good, I appreciate the leaner API. You mentioned that it is now possible to keep the H5 file open during the simulation, eliminating some issues we were having. Does the current implementation already make use of this? if not I can work it out in a different branch.

Yest, this is already possible, e.g. as highlighted in this test

https://github.com/zincware/ZnH5MD/blob/5657dd67a150a2afef704e3f26d2d80df0071a6a/tests/test_datasets.py#L109-L113

but my suggestion would be:

  1. release znh5md v0.3
  2. update to final release and merge
  3. create a different PR and do a performance comparison which might lead to improvements in znh5md in the making

What do you think?

@M-R-Schaefer
Copy link
Contributor

sounds good

@PythonFZ PythonFZ marked this pull request as ready for review July 16, 2024 08:04
@PythonFZ PythonFZ merged commit b85f0cd into dev Jul 16, 2024
1 of 2 checks passed
@PythonFZ PythonFZ deleted the bump_h5md branch July 16, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants