NikolausDemmel / rootba

Square Root Bundle Adjustment for Large-Scale Reconstruction
BSD 3-Clause "New" or "Revised" License
291 stars 37 forks source link

Shared intrinsics #11

Open reynoldscem opened 2 years ago

reynoldscem commented 2 years ago

Hi, is it possible to share intrinsics between different cameras, or must they all be independent?

NikolausDemmel commented 2 years ago

It's not implemented in this code base, but in principle, it can be done with Square Root BA. It just means you would have fewer intrinsics columns in your landmark blocks.

pyramidpoint commented 1 year ago

@NikolausDemmel hello, is it possible to fix intrinsics when ba?

NikolausDemmel commented 1 year ago

Yes, but it’s not implemented as of now.

A simple way is to set the updates to intrinsically to zero.

A better way, but with more code changes, is dropping the columns from the landmark blocks.

NikolausDemmel commented 1 year ago

A simple way is to set the updates to intrinsically to zero.

(and Jacobisn columns)

pyramidpoint commented 1 year ago

Thanks for your reply, but I don't know how to do through the better way, I shield the apply_inc_intrinsics function(https://github.com/NikolausDemmel/rootba/blob/d3900037fc4bc98328e5d8d26f1c4eed922f88cc/src/rootba/solver/linearizor_power_sc.cpp#L188) when updating cameras, and I look like it works fine, is it true?

NikolausDemmel commented 1 year ago

It should work ok, this way you ensure the intrinsics are never updated.

You might get slightly better convergence if you also zero out the increment for the intrinsics before calling lsc_->back_substitute(inc). And even better, if you set the Jacobian columns w.r.t. intrinsics to zero in the landmark blocks (except for the damping rows). Without that, the remaining parameters (landmarks, camera poses) will get an update that assumes the intrinsics are also updating. But since the intrinsic change in each iteration is usually small and you iterate, I guess in practical terms it might not make a significant difference.