Skip to content

Conversation

fabiencastan
Copy link
Member

New notion of local isolated computation for python nodes using meshroom_compute.

Reorganization

  • BaseNode: is the base class for all nodes
  • Node: is now dedicated to python nodes, with the implementation directly in the process function
  • CommandLineNode: dedicated to generate and run external command lines

- add option to control verbosity
- in extern mode: disable logging and enable it only for the node
computation (to avoid polluting the node's log with general warnings)
…oom_compute

Reoganization
- BaseNode: is the base class for all nodes
- Node: is now dedicated to python nodes, with the implentation directly
in the process function
- CommandLineNode: dedicated to generate and run external command lines
Avoid qml errors when accessing non-existing nodes.
The entry exist in the dict and we retrieve a null node.
@fabiencastan fabiencastan force-pushed the dev/isolatedProcessEnv branch from 0ee1757 to 426855b Compare March 24, 2025 17:12
@fabiencastan fabiencastan force-pushed the dev/isolatedProcessEnv branch 2 times, most recently from 39b48cc to 346d78d Compare April 13, 2025 17:08
@fabiencastan fabiencastan requested a review from Copilot April 14, 2025 08:13
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 20 out of 23 changed files in this pull request and generated 2 comments.

Files not reviewed (3)
  • meshroom/ui/qml/Application.qml: Language not supported
  • meshroom/ui/qml/GraphEditor/GraphEditor.qml: Language not supported
  • meshroom/ui/qml/GraphEditor/Node.qml: Language not supported
Comments suppressed due to low confidence (3)

meshroom/submitters/rippleSubmitter.py:1

  • Ensure that the removal of rippleSubmitter.py is intentional and verify that there are no remaining dependencies on this module.
Entire file removed

meshroom/core/graph.py:1384

  • Reassigning cacheDir to an empty string may bypass the intended default cache folder behavior; please verify that this change is deliberate.
self.cacheDir = ""

meshroom/core/nodeFactory.py:171

  • [nitpick] Consider resolving or documenting the potential naming conflicts between user-defined inputs/outputs and internal names to avoid ambiguity.
# TODO: user inputs/outputs may conflicts with internal names (like position, uid)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@fabiencastan fabiencastan requested a review from cbentejac April 14, 2025 08:29
@fabiencastan fabiencastan force-pushed the dev/isolatedProcessEnv branch from cb15a0b to 2ad5535 Compare April 14, 2025 09:24
Copy link

codecov bot commented Apr 14, 2025

Codecov Report

Attention: Patch coverage is 55.97610% with 221 lines in your changes missing coverage. Please review.

Project coverage is 76.75%. Comparing base (bb9df08) to head (f8567fb).
Report is 54 commits behind head on develop.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
meshroom/core/node.py 60.00% 108 Missing ⚠️
meshroom/core/desc/node.py 34.06% 60 Missing ⚠️
meshroom/core/__init__.py 44.70% 47 Missing ⚠️
meshroom/core/graph.py 86.11% 5 Missing ⚠️
meshroom/env.py 91.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2703      +/-   ##
===========================================
- Coverage    79.01%   76.75%   -2.27%     
===========================================
  Files           39       40       +1     
  Lines         5809     6082     +273     
===========================================
+ Hits          4590     4668      +78     
- Misses        1219     1414     +195     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

"isComputable" is renamed as "isComputableType": this function is only
about the Meshroom Node type and not about the computability in the
current context.

Even if we are in compatibility mode, we may has access to the nodeDesc
and its information about the node type.
When the compatibility node was fully computed, the recompute/resubmit
button was available in the UI menu.
The reason is that we need to propagate that the connected nodes can be
computed if there is a compatibility node in the dependency iff it is
already fully computed.
Now there is an additional explicit check.
The states of nodes were not valid the first time they were loaded, but
became valid after any topological update (such as creating or removing
a node or edge).
The reason was that the loading of the graph was done before setting the
project path (and thus the cacheDir). So the status files of the nodes
were not available during the graph creation and were not updated at the
end of the load.
Now we set the filepath/cache first, so everything is right from the
start.
@cbentejac cbentejac added this to the Meshroom 2025.1.0 milestone Apr 28, 2025
@cbentejac cbentejac force-pushed the dev/isolatedProcessEnv branch from d25b830 to f8567fb Compare April 28, 2025 16:25
@cbentejac cbentejac merged commit 006288a into develop Apr 28, 2025
2 of 5 checks passed
@cbentejac cbentejac deleted the dev/isolatedProcessEnv branch April 28, 2025 16:31
@fabiencastan fabiencastan mentioned this pull request Jun 7, 2025
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants