-
Notifications
You must be signed in to change notification settings - Fork 70
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
Make the analysis storage chunk equal to checkpoint interval in SAMS #912
Conversation
This might need some more refinement to this to prevent the mpi file locking from hdf5 1.10.x versions. We cannot use |
Adding discussion from slack conversation for documentation. From @jchodera:
We can still accept this PR, of course. It will likely be much better than no chunking! From @Lnaden
I don’t think we should do that, I’m worried we would run into a memory problem at that point if we are not careful. I do think the
There are no hard and fast rules sadly. The chunk sizes trade off fixing slow access of large data while slowing down fast access of small data. Maybe we should all sit down and brainstorm some more about what the correct chunksize is for the iteration dimension. Either way, its now a single variable we can tune in the I’ll merge the PR anyways and we can refine it later as needed |
@jchodera is correct in that the memory argument is not a good one. It is not memory limited. Assuming chunksize of all iterations, there are a few problems I see:
Also, the very large chunksizes will slow down all write actions too since the whole datset must be loaded for each write access too. See the HDFGroup's whitepaper on too large of chunksizes (a good read)
|
Great find! Ok, let's go with this for now! |
We'll want to optimize this some more. People with large checkpoint intervals may slow down their write speeds at runtime. So maybe set a maximum size, just as an idea. |
This is an optimization for read/write by using the
checkpoint_interval
for the analysis file, and 1 for the checkpoint file.
Supersedes and closes #861 by merging into SAMS
Fixes #860