-
Notifications
You must be signed in to change notification settings - Fork 43
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
Zarr sink Improvements #1713
Zarr sink Improvements #1713
Conversation
I have a multiprocess code path that behaves poorly. In the main process, create the sink and add a tile. I think the solution is that if My test case was to use the
and then |
Hmm... If we did that, then maybe we'd also need to call |
I took some time to observe the behavior of your example use case in multiple python versions and plot the output from For all of these python versions, the only spike in memory I experienced was in the main process after the multiprocessing stage, during the conversion step of the In my experience, 3.10 performed the best and 3.11 performed the worst. Am I remembering correctly that you said you tried this with 3.11? If my 3.11 graph does not reflect what you experienced, can you send me the full list describing your environment so I can do my best to replicate? Otherwise, I'm not sure that we can do much to further improve memory usage, and this may just be a matter of python and other library versions. EDIT: Here's the command I ran in each environment:
|
Also, write the metadata after fundamental changes so that anything that reopens the file after that point will see those changes.
When reopening a zarr sink, validate it
@annehaley I think the only thing left is to document that if you use multi-processes and don't pre-allocate the maximal dimension, axes values aren't fully recorded. |
We are getting some transient failures in CI. It doesn't have anything directly to do with this PR. I can erratically replicate it locally on master. We'll rerun CI until it passes for this PR and a different PR will address the flaky test. |
If #1756 is merged to master before this passes, we can just rebase off of master and it should pass. |
Resolves #1674. Resolves #1698.