Add BinaryTreePartition class for hierarchical image-pair partitioning #840
+443
−24
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR introduces improvements to the graph partitioning functionality in GTSFM by adding new partitioner implementations, visualization support, and corresponding tests. Key changes include:
- Updated import ordering and added a commented-out alternative partitioner configuration in gtsfm_runner_base.py.
- Addition of new partitioner implementations in eight_partitions.py and binary_tree_partition.py, along with visualization (binary_tree_partition_visualization.py) and tests (binary_tree_partition_test.py).
- Updated environment file (environment_m3_ultra.yml) to include new dependencies such as metis and graphviz.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gtsfm/runner/gtsfm_runner_base.py | Revised import ordering and added an alternative partitioner option. |
| gtsfm/graph_partitioner/eight_partitions.py | Introduced a partitioner using a 3-level binary tree with METIS. |
| gtsfm/graph_partitioner/binary_tree_partition_visualization.py | Added visualization support for binary tree partitions. |
| gtsfm/graph_partitioner/binary_tree_partition_test.py | Added tests for the binary tree partitioner. |
| gtsfm/graph_partitioner/binary_tree_partition.py | Implemented binary tree partitioning using METIS ordering. |
| environment_m3_ultra.yml | Updated environment file with new dependencies. |
Comments suppressed due to low confidence (1)
gtsfm/runner/gtsfm_runner_base.py:292
- [nitpick] The commented alternative partitioner uses 'EightPartition()', but the implemented class is named 'EightPartitions'. Update the comment for consistency.
# graph_partitioner = EightPartition()
|
@tiantianxiangshang629 status of this PR? |
fe3e323 to
879f35a
Compare
| @@ -90,7 +89,6 @@ def __init__( | |||
| self._pose_angular_error_thresh = pose_angular_error_thresh | |||
| self.output_root = Path( 8000 output_root) | |||
| self._output_worker = output_worker | |||
| self._create_output_directories() | |||
There was a problem hiding this comment.
why is this removed?
There was a problem hiding this comment.
I implemented a new create_output_directories function to create the partition result directories.
| else: | ||
| raise ValueError(f"Unsupported COLMAP camera type: {camera_model_name}") | ||
|
|
||
| intrinsics_gtsfm.append(Cal3Bundler(fx, k1, k2, cx, cy)) | ||
| if camera_model_name == "OPENCV": |
There was a problem hiding this comment.
were you able to run gtsfm with Cal3DS2? I think it was not as good as Cal3Bundler when we have used it before.
There was a problem hiding this comment.
Yes, I was. The reason for using it is that Cal3Bundler does not accept those distortion arguments from OPENCV.
There was a problem hiding this comment.
How did you get it to run with Cal3DS2 without adding Cal3DS2 support to the bundle adjustment module? Only Cal3Bundler and Cal3Fisheye are supported I believe.
6c4dfe3 to
fd306e3
Compare
Remove draft code. Add binayr tree partition result. Add OPENCV camera model. Untrack binary_tree_partition_visualization.py and update .gitignore Remove unused log. Fix format issue. Add docstring to binary tree partition. Move unittest under test folder. Remove incorrect unittest file. Use more informative function name. Add unittests for member functions. Fix import format. Remove tracked binary files and local image dataset Remove temp scripts and image assets from version control Remove points3D.txt from tracking
d509a4a to
57772a0
Compare
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.
This PR introduces a new BinaryTreePartition class to the GTSFM graph partitioning module. The partitioner supports hierarchical binary tree-based splitting of image-pair graphs, enabling configurable partition depths for scalable and efficient scene processing.
Key Features: