Skip to content
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

2 stage filtering in ViewGraphEstimator #739

Merged
merged 5 commits into from
Mar 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gtsfm/configs/unified.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ SceneOptimizer:
# comment out to not run
view_graph_estimator:
_target_: gtsfm.view_graph_estimator.cycle_consistent_rotation_estimator.CycleConsistentRotationViewGraphEstimator
edge_error_aggregation_criterion: MEDIAN_EDGE_ERROR
edge_error_aggregation_criterion: MIN_EDGE_ERROR

rot_avg_module:
_target_: gtsfm.averaging.rotation.shonan.ShonanRotationAveraging
Expand Down
27 changes: 25 additions & 2 deletions gtsfm/multi_view_optimizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@
from gtsfm.common.keypoints import Keypoints
from gtsfm.common.pose_prior import PosePrior
from gtsfm.common.sfm_track import SfmTrack2d
from gtsfm.common.two_view_estimation_report import TwoViewEstimationReport
from gtsfm.data_association.cpp_dsf_tracks_estimator import CppDsfTracksEstimator
from gtsfm.data_association.data_assoc import DataAssociation
from gtsfm.evaluation.metrics import GtsfmMetricsGroup
from gtsfm.view_graph_estimator.cycle_consistent_rotation_estimator import (
CycleConsistentRotationViewGraphEstimator,
EdgeErrorAggregationCriterion,
)
from gtsfm.view_graph_estimator.view_graph_estimator_base import ViewGraphEstimatorBase
from gtsfm.data_association.cpp_dsf_tracks_estimator import CppDsfTracksEstimator
from gtsfm.common.two_view_estimation_report import TwoViewEstimationReport


class MultiViewOptimizer:
Expand All @@ -44,6 +48,10 @@ def __init__(
self.ba_optimizer = bundle_adjustment_module
self._run_view_graph_estimator: bool = self.view_graph_estimator is not None
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use the flag for stage 2 too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ayushbaid did you mean to add the MEDIAN_EDGE_ERROR stage to the config also?

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant to make it optional. If we do not want to run stage 2 we can easily skip it.

But if we get good performance across the board and there is no reason to not use it, I am fine.


self.view_graph_estimator_v2 = CycleConsistentRotationViewGraphEstimator(
edge_error_aggregation_criterion=EdgeErrorAggregationCriterion.MEDIAN_EDGE_ERROR
)

def __repr__(self) -> str:
return f"""
MultiviewOptimizer:
Expand Down Expand Up @@ -114,6 +122,21 @@ def create_computation_graph(
two_view_reports_dict,
debug_output_dir,
)
(
viewgraph_i2Ri1_graph,
viewgraph_i2Ui1_graph,
viewgraph_v_corr_idxs_graph,
viewgraph_two_view_reports_graph,
viewgraph_estimation_metrics,
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you save these metrics as a separate one, so we have numbers for stage1 and stage2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IMHO this will be overly verbose. We already get visualizations saved for each of the two stages, currently, and I think the combined performance is what matters most.

) = self.view_graph_estimator_v2.create_computation_graph(
viewgraph_i2Ri1_graph,
viewgraph_i2Ui1_graph,
all_intrinsics,
viewgraph_v_corr_idxs_graph,
keypoints_list,
viewgraph_two_view_reports_graph,
debug_output_dir / "2",
)
else:
viewgraph_i2Ri1_graph = dask.delayed(i2Ri1_dict)
viewgraph_i2Ui1_graph = dask.delayed(i2Ui1_dict)
Expand Down
Loading