-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[core] Add support for dynamic chunks #2918
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
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (51.55%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2918 +/- ##
===========================================
- Coverage 80.74% 78.91% -1.83%
===========================================
Files 59 59
Lines 7853 8305 +452
===========================================
+ Hits 6341 6554 +213
- Misses 1512 1751 +239 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
02bae2f to
15df47f
Compare
4b47850 to
51cfd91
Compare
15b7db5 to
ecbcbed
Compare
cbentejac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional notes:
- Let's say I compute a graph (for example, the photogrammetry template) in a given location. All my nodes have been successfully computed, the statuses are all set to "SUCCESS". Now let's say I do a "save as" on the graph I just computed and save it to a new location. The cache changes, so we expect all the nodes' status to be reset. For nodes that have dynamic chunks, the display of the status is not refreshed, and we end up with nodes that appear as computed, although they have lost all their status files.
Below is a screenshot of the photogrammetry graph that was computed and saved in a new location (the first branch was computed, the second was added from the template for the sake of comparison):
-
There are some unpredictable behaviours when performing actions that are allowed but probably shouldn't be. An example would be clicking on "stop task" for a task that is already done computing and is in "SUCCESS" state, while nodes are being computed later on in the graph. The UI allows to perform it, there is an info message saying that the task has been successfully stopped, and the node's status is updated to "STOPPED" in the task manager and Graph Editor; it does not enable the "resume job" button. This seems to cause the job on the farm to finish computing the current task and then pausing the rest of the job (to be verified).
-
I have noticed on several occasions that when clicking on buttons from the "JOB" tab, the info message that is sent contains the name of a node that is not part of the job at all (if all my submitted tasks are for NodeName_2, I may get a message about NodeName_1, which is not being computed at all).
cbentejac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional notes:
- Let's say I compute a graph (for example, the photogrammetry template) in a given location. All my nodes have been successfully computed, the statuses are all set to "SUCCESS". Now let's say I do a "save as" on the graph I just computed and save it to a new location. The cache changes, so we expect all the nodes' status to be reset. For nodes that have dynamic chunks, the display of the status is not refreshed, and we end up with nodes that appear as computed, although they have lost all their status files.
Below is a screenshot of the photogrammetry graph that was computed and saved in a new location (the first branch was computed, the second was added from the template for the sake of comparison):
-
There are some unpredictable behaviours when performing actions that are allowed but probably shouldn't be. An example would be clicking on "stop task" for a task that is already done computing and is in "SUCCESS" state, while nodes are being computed later on in the graph. The UI allows to perform it, there is an info message saying that the task has been successfully stopped, and the node's status is updated to "STOPPED" in the task manager and Graph Editor; it does not enable the "resume job" button. This seems to cause the job on the farm to finish computing the current task and then pausing the rest of the job (to be verified).
-
I have noticed on several occasions that when clicking on buttons from the "JOB" tab, the info message that is sent contains the name of a node that is not part of the job at all (if all my submitted tasks are for NodeName_2, I may get a message about NodeName_1, which is not being computed at all).
8a599b4 to
e720c8b
Compare
dede3a7 to
4f96f9d
Compare
b371eef to
22ae573
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a significant architectural change to the Meshroom chunk computation system, allowing nodes to delay chunk creation until computation time. This enables support for nodes without predetermined sizes and adds enhanced job management capabilities for render farm submissions.
Key Changes:
- Introduces delayed chunk evaluation with
_chunksCreatedflag and placeholder chunks for nodes with dynamic sizes - Adds new computation levels (
EXTREMEandSCRIPT) for better resource targeting - Implements
NodeStatusDatafor tracking node-level status separate from chunk status - Introduces comprehensive job management API with
BaseSubmittedJobinterface andJobManagerfor controlling farm jobs - Creates new
meshroom_createChunksscript for handling chunk initialization during farm submission
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
meshroom/ui/reconstruction.py |
Adds safety check for empty chunks list before connecting signals |
meshroom/ui/qml/Utils/Colors.qml |
Adds new color (darkpurple) and INPUT status color, plus getNodeColor() function |
meshroom/ui/qml/GraphEditor/TaskManager.qml |
Major UI overhaul with task/job toolbars and chunk selection support |
meshroom/ui/qml/GraphEditor/NodeEditor.qml |
Handles chunk visibility based on chunksCreated flag |
meshroom/ui/qml/GraphEditor/NodeChunks.qml |
Improves null safety for chunk model handling |
meshroom/ui/qml/GraphEditor/Node.qml |
Adds chunk placeholder display for nodes without created chunks |
meshroom/ui/qml/GraphEditor/GraphEditor.qml |
Adds context menu items for external job control (interrupt, retry) |
meshroom/ui/qml/GraphEditor/ChunksListView.qml |
Syncs chunk selection from reconstruction to list view |
meshroom/ui/qml/Controls/NodeActions.qml |
Enhances action buttons with retry and improved submit/stop states |
meshroom/ui/qml/Application.qml |
Makes submit button toggle between submit and interrupt based on execution state |
meshroom/ui/graph.py |
Refactors ChunksMonitor to NodeStatusMonitor for both node and chunk monitoring |
meshroom/core/taskManager.py |
Implements chunk creation in TaskThread with signal-based coordination |
meshroom/core/submitter.py |
Introduces new submitter API with BaseSubmittedJob and JobManager |
meshroom/core/node.py |
Separates node status (NodeStatusData) from chunk status (ChunkStatusData) and implements delayed chunk creation |
meshroom/core/graph.py |
Updates graph processing to handle nodes without initialized chunks |
meshroom/core/desc/node.py |
Adds ExitCleanup handler for subprocess management |
meshroom/core/desc/computation.py |
Adds EXTREME and SCRIPT computation levels |
meshroom/core/__init__.py |
Improves submitter loading error messages |
meshroom/__init__.py |
Adds MeshroomExitStatus enum for special exit codes |
bin/meshroom_createChunks |
New script for chunk creation and task spooling during farm submission |
bin/meshroom_compute |
Updates to handle nodes without created chunks and status checking |
.git-blame-ignore-revs |
Adds linting commits to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for node in nodesToProcess: | ||
| node.destroyed.connect(lambda obj=None, name=node.name: self.onNodeDestroyed(obj, name)) | ||
| node.initStatusOnSubmit() # update node status | ||
| # TODO : Notify the node that there was an issue on submit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to do
| else: | ||
| for node in nodesToProcess: | ||
| node.initStatusOnSubmit() # update node status | ||
| # TODO : Notify the node that there was an issue on submit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to do
`nodeType` and `packageName` are already part of the node's status file, and there is thus no need to propagate them to the chunks'.
…Blocks in status files
`canBeStopped` and `canBeCanceled` only took into account nodes that were being run locally. As nodes that have been submitted externally (exclusively on a render farm, not in another instance of Meshroom) can now be stopped and canceled as well, they are considered in both methods.
When a job has been submitted, the state of the `Submit` button switches to `stoppable`. Clicking it in this state interrupts the node's computation on the farm.
`chunkPlaceholder` is a list model that is meant to contain a placeholder chunk for nodes that have dynamic chunks: prior to the chunks' creation, it is used to reflect the state of the node as a single chunk. In particular, this allows to reflect accurately the node's status while it is submitted but the chunks have not been created yet. The property is a list model as it allows it to be used as a model later on on the QML side.
`onGraphUpdated` updates the models for chunks, including the placeholder, which allows to have an up-to-date status for all the existing chunks as well as the placeholder one when a node's number of chunks hasn't been determined yet.
d1abeb6 to
38211d6
Compare
Description
This PR goal is to change chunk system to be able to work with graphs where nodes can have no specific size set.
Changes
Compute system
extreme(that targets higher specs that what intensive does)scriptwhich is a mode for single-process simple process. It targets specific machines on the farm that have the ability to run multiple jobs in parallel so for simple jobs it should allow faster processingnode_chunksCreatedparam has been added to check if the chunks have been correctly initializedresetChunkswhen loading meshroom, that will create a list with a single chunk, and we can create chunks at any moment with_createChunks.NodeStatusDatahas been added that tracks anodeStatusfile which is similar to the chunk statuses files but is specific to the node. This node status is used to get the cached range parametrization of the node so that we can retrieve it when possible with_createChunksFromCache.taskManagerTaskThreadandTaskManagerhave been modified to launch the chunk creation when the node compute startsSubmitters
BaseSubmitterAPI have been updated to allow more flexibility. A newBaseSubmittedJobexists and is used to track jobs that have been created. The goal is to use this as an interface to call actions on it (stop/pause/restart...)bin/meshroom_createChunksscript have been created, and handles the chunk creation and additional chunk tasks spoolingChanges to the tractor API have been implemented here meshroomHub/mrSubmitters#1
Examples