prepare_9um_isotropic_input: publish the staged zarr in a way Windows… - #1663
Closed
khj1222 wants to merge 1 commit into
Closed
prepare_9um_isotropic_input: publish the staged zarr in a way Windows…#1663khj1222 wants to merge 1 commit into
khj1222 wants to merge 1 commit into
Conversation
… survives The script writes every tile into <output>.zarr.partial and then renames it. On Windows that rename raises PermissionError WinError 5 while any file inside the staging directory is still open, and one handle is enough -- measured: renaming succeeds with nothing held, and fails with exactly WinError 5 with a single file inside opened for read. After writing a multi-gigabyte zarr, something holding a handle briefly is ordinary. The failure cost more than the rename. Every tile was already on disk, but the output was a bare traceback pointing at pathlib, so the obvious reading was that hours of conversion had been lost and had to be repeated. Release the store's handles before renaming, retry the rename with backoff, and if it still fails say what is true: the conversion finished, nothing needs recomputing, and a manual rename completes it. Verified on Windows: publishes immediately with nothing held; retries and succeeds when a handle is released after 0.8 s; and with a handle never released, exits with that message and leaves the staging directory intact. End to end on a synthetic 84x300x260 volume the output is byte-identical to the expected pooling.
|
@khj1222 is attempting to deploy a commit to the scroll Team on Vercel. A member of the Team first needs to authorize it. |
1 task
Contributor
Author
|
Closing in favour of a version against |
1 task
Contributor
Author
|
Superseded by #1705. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In one sentence: A finished 9 µm conversion is no longer thrown away by the last line of the script on Windows.
One real example: Preparing the aligned inputs for the
ink_9umcorpus,prepare_9um_isotropic_input.pywrote all 416 tiles ofphercparis4-w00and then died renaming the staging directory onto the output path:Before: the data was complete and on disk, but the script exited non-zero with a traceback that points at
pathlib. The obvious reading is that the conversion has to be repeated; the actual remedy is a manual rename. Across 24 inputs this happened often enough that I wrote a driver to finish the renames.After this PR: the store's handles are released before the rename, the rename is retried with backoff, and if it still cannot publish the script says so in the terms that matter:
Proof: the cause first — a directory rename on Windows, one condition changed at a time:
os.scandiron the directorySo WinError 5 here means a file inside is still open — which is what an indexer or scanner does for a moment after a large write, and why a short retry is the right shape of fix rather than a louder failure.
Then the function, on Windows:
SystemExitwith the message above; staging directory left intactAnd end to end, on a synthetic 84×300×260 uint8 volume (84 = 21 ×
POOL_Z): exit 0, output shape(21, 300, 260), and the bytes are identical to the expected rounded mean pooling. No staging directory left behind.Why / where this is useful: anyone preparing aligned inputs on Windows, which is the first step of the ink recipe. The cost of the bug is not the rename — it is that a completed multi-gigabyte conversion looks lost.
Details
Three changes in one file:
output_shapeis captured andtarget/groupdeleted before the rename, so the store is not holding the directory it is about to move. (The finalprintusedtarget.shapeafter the rename; it now uses the captured value.)publish_partial()retriesPath.replacesix times with exponential backoff from 0.5 s.SystemExitwith both paths, the OS error, the reason, and the fact that nothing needs recomputing.POSIX renames a directory whose files are open, so this is invisible on Linux and macOS; nothing changes there beyond one
deland a loop that succeeds on its first attempt.Scope:
mainfactors this intoink_detection/preprocessing/staged_write.py:publish_staged_output, shared byclean_labels,composite_from_zarrandmerge_predictions. The same exposure applies wherever a directory is published rather than a file. I have kept this PR to the script whose failure I actually hit; happy to follow up there if you want the helper hardened too.Why this matters to me: I was preparing 24 alignment inputs at 9.6 µm, and phercparis4-w00 made it all the way through tile 416/416 before failing on the final rename with a PermissionError. Since the only output was a pathlib traceback, I initially thought the whole multi-hour conversion had been lost. It turned out all of the data was already on disk and only the rename step had failed, so I ended up writing a small script just to finish that rename separately.