Skip to content

BA updates - save covariances, grid size -> patch size, more config options#1101

Open
akshay-krishnan wants to merge 6 commits intomasterfrom
ba-updates
Open

BA updates - save covariances, grid size -> patch size, more config options#1101
akshay-krishnan wants to merge 6 commits intomasterfrom
ba-updates

Conversation

@akshay-krishnan
Copy link
Copy Markdown
Collaborator

@akshay-krishnan akshay-krishnan commented Apr 3, 2026

  • Add functionality to compute and save the covariances using gtsam.Marginals. This is only for debugging, not for use in the algorithm.
  • Min track length pre-BA should be 3.
  • Do not BA cameras that have < 20 tracks. What do we we do with them? retain the pre-BA camera poses as these could be important to merge children upstream. Should we also retain the tracks? discarding them now, could revisit later if that causes problems.
  • change from grid-size to patch size for filtering tracks. grid-size is unclear, perhaps this was using recangular patches before?
  • expose some more options to customize BA, eg. relative cost tolerance.

@akshay-krishnan akshay-krishnan changed the title BA updates BA updates - save covariances, grid size -> patch size, more config options Apr 3, 2026
initial_data, graph, self._ordering_type
)
if self._compute_pose_covariances:
try:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nothing wrong here, but just confused that we are taking the covariance matrices from the cameras_to_model and then set it in optimized data which is the recovered after optimization so should have those inherently?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

are you suggesting moving this to __optimize_and_recover ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no I am just a little confused on what this if block does

@hkhanuja
Copy link
Copy Markdown
Collaborator

hkhanuja commented Apr 4, 2026

Just thinking if it should be called number of tracks per camera or number of measurements per camera? and I think if we have 2 cameras with less than 20 measurements after all pre-ba filtering, we shouldn't remove those tracks because those tracks might have measurements in other cameras (maybe helpful in merging ahead)

@akshay-krishnan
Copy link
Copy Markdown
Collaborator Author

Just thinking if it should be called number of tracks per camera or number of measurements per camera? and I think if we have 2 cameras with less than 20 measurements after all pre-ba filtering, we shouldn't remove those tracks because those tracks might have measurements in other cameras (maybe helpful in merging ahead)

we dont remove the tracks, we remove the camera (which removes the measurements during BA). tracks_per_camera or measurements_per_camera is the same since it is defined for a particular camera (tracks == measurements). while measurements is more accurate, tracks is a simpler name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants