Give PoissonRecon a writable temporary directory - #2056
Conversation
|
@MJohnson459 and / or @DodgySpaniard -- can you review the test? The code change looks fine, and very happy to see the addition of a test. |
There was a problem hiding this comment.
Thanks! The change looks good but if you don't mind I think it would be better to put the scratch files into a subdirectory, just to make it easier to clean up later. The PoissonRecon call is flaky and we don't want to create a load of junk files. Something like:
# ... existing outMeshDirty setup ...
# PoissonRecon writes out-of-core PR_* files to $TMPDIR, falling back to the
# working directory, which may be read-only even when mesh_path is writable.
tempdir = os.path.join(mesh_path, "%s-tmp" % basename)
if os.path.exists(tempdir):
log.ODM_WARNING("Removing previous PoissonRecon temp directory: %s" % tempdir)
shutil.rmtree(tempdir)
os.makedirs(tempdir)
...
# After the loop
if os.path.exists(tempdir):
shutil.rmtree(tempdir)For the test, honestly I would just remove it. It doesn't really test anything not explicitly already in the source, only that --tempDir is appended to the command line. There is no logic in the code and it wouldn't catch the bug that triggered this issue. On the flip side you could argue any test is better than no test. @smathermather your call.
Problem
PoissonReconstores its out-of-corePR_*scratch files in--tempDir, then$TMPDIR, then the process working directory. ODM already knows a writable mesh output directory, but did not pass it to PoissonRecon.This makes an otherwise resumable mesh stage fail when ODM is launched by a service manager with a non-writable working directory (for example
/). Reducing the Poisson thread count cannot repair that filesystem failure, so every retry fails beforeReconstructMeshsees a dirty mesh.Change
Pass the absolute output-mesh directory to PoissonRecon through its existing
--tempDiroption. This keeps scratch files beside the dirty mesh and leaves the public Python API and retry behavior unchanged.A focused unit test captures the generated commands and covers a writable output path containing a space.
Verification
$TMPDIR/./fallback andmkstempuse, then independently recovered in a 24,136,575-point run by supplying a writable working directory.No container-specific behavior or new dependency is introduced.