-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Fix] Prevent nested cache creation when using --save and --cache #2890
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
base: develop
Are you sure you want to change the base?
Conversation
When running `meshroom_batch` with both `--save` and `--cache` arguments simultaneously, a nested cache directory is created. This prevents subsequent nodes from finding their inputs, causing the pipeline to fail. The root cause is a redundant `graph.save()` call in `executeGraph()` that defaults to `setupProjectFile=True`. This re-triggers an automatic cache setup logic which conflicts with the explicit `--cache` path. This commit fixes the issue by changing the call to `graph.save(setupProjectFile=False)`. This disables the redundant and problematic setup logic while preserving the intended functionality of saving the graph's progress during computation. Issue Number alicevision#2889
@jikbb2 Thanks for the PR! Or do you have a need in your use cases to put the cache and the project file in different places? |
I've checked the cache is not stored in the project. So when you will reload the computed Meshroom scene, it will not find the MeshroomCache. |
Hi @fabiencastan , Thanks for your feedback. I'd like to clarify the implementation to make sure I get it right. Which of these two options do you prefer? Disallow simultaneous use: Add a check to prevent using Completely remove: Remove the Additionally, should this PR also cover updating the command's help text and any other user-facing descriptions to reflect the change? |
Hi @jikbb2 , |
This merges the 'fix/nested-cache' branch, which contains the complete removal of the --cache option, into 'patch-1' to update the open pull request.
I've updated the PR according to your feedback. The --cache option has been completely removed from meshroom_batch as requested. Thanks again for the guidance! @fabiencastan |
Description
This PR fixes a critical bug where running
meshroom_batch
with both--save
and--cache
arguments simultaneously creates a nested cache directory (e.g.,test_cache/test_cache
).This incorrect structure is not just a cosmetic issue; it breaks the pipeline execution. Subsequent nodes fail to locate their input files from previously completed nodes because they search in the expected (non-nested) path instead of the actual nested one. This causes a failure to reference intermediate results and halts the computation.
Features list
Implementation remarks
The root cause was a redundant
graph.save()
call at the beginning ofexecuteGraph()
inmeshroom/core/graph.py
. This call used the default argumentsetupProjectFile=True
, which incorrectly re-triggered the project's automatic cache setup logic, conflicting with the user-provided--cache
argument.The implemented solution is a minimal, targeted change: the problematic call is modified to
graph.save(setupProjectFile=False)
.This preserves the intended functionality of saving the graph's progress during computation while disabling the specific logic that caused the path conflict. This ensures the user-provided cache path is respected as the single source of truth.
To test this fix:
Follow the "Steps to Reproduce" from issue #ISSUENUMBER.
Run the command:
meshroom_batch -p photogrammetry -i ./images --save test.mg --cache test_cache
.Verify that a single, non-nested
test_cache
directory is created and the pipeline completes successfully without errors.