OpenSfM icon indicating copy to clipboard operation
OpenSfM copied to clipboard

BundleShotPoses possibly broken

Open kielnino opened this issue 1 year ago • 8 comments

I have a simple setup with some images to reconstruct, all taken by the same camera. As I understand the code, individual shots are added to a fake rig instance that only contains the one shot.

In the reconstruction step grow_reconstruction the newly added shots are passed to bundle_shot_poses.

As the newly added shot has the fake rig_instance_id with the same id as the shot_id, rig_instances_ids now only contains the newly added shot. In Line 481 the rig_instances_ids are iterated and the corresponding shots are added to the bundle problem (Line 516).

As part of this it is checked if the found shot_id is in the list of the newly-added shots (which are supposed to be bundled), and this is always the case. So all shots are fixed and the BA only consists of constant blocks. See the ceres summary full-report:

Solver Summary (v 2.1.0-eigen-(3.4.0)-lapack-eigensparse-no_openmp)

                                     Original                  Reduced
Parameter blocks                           67                        0
Parameters                                216                        0
Residual blocks                            65                        0
Residuals                                 140                        0

Minimizer                        TRUST_REGION

Sparse linear algebra library    EIGEN_SPARSE
Trust region strategy     LEVENBERG_MARQUARDT

                                        Given                     Used
Linear solver                        DENSE_QR   SPARSE_NORMAL_CHOLESKY
Threads                                    20                       20
Linear solver ordering              AUTOMATIC                AUTOMATIC

Cost:
Initial                          9.919183e-01
Final                            9.919183e-01
Change                           0.000000e+00

Minimizer iterations                       -2
Successful steps                           -1
Unsuccessful steps                         -1

Time (in seconds):
Preprocessor                         0.000770

  Residual only evaluation          -1.000000 (-1)
  Jacobian & residual evaluation    -1.000000 (-1)
  Linear solver                     -1.000000 (-1)
Minimizer                            0.000000

Postprocessor                        0.000002
Total                                0.000772

Termination:                      CONVERGENCE (Function tolerance reached. No non-constant parameter blocks found.)

As I see this, the newly added shot is always set to fixed and the complete bundle of the newly added shots-poses never work. How the whole thing works when real rigs are used, I can not foresee, but even with single shots the BundleShotPoses should do something.

kielnino avatar Dec 11 '24 10:12 kielnino

@fabianschenk , could you take a look at this when you have time?

kielnino avatar Dec 11 '24 10:12 kielnino

Steps to reproduce:

  1. Add the line LOG(INFO) << ba.FullReport(); at 564
  2. run bin/opensfm_run_all data/berlin

kielnino avatar Dec 12 '24 07:12 kielnino

@paulinus Is there any chance that one of the project members could take a look at this? Unnecessary ceres runs may have a performance impact on any reconstruction.

kielnino avatar Apr 19 '25 11:04 kielnino

From how I understand it, maybe it's simple as changing Line 501 from

const auto is_fixed = shot_ids.find(shot_id) != shot_ids.end();

to

const auto is_fixed = shot_ids.find(shot_id) == shot_ids.end();

which would set all shots in the reconstruction to fixed, except those passed in the shot_ids parameter.

kielnino avatar May 07 '25 12:05 kielnino

@YanNoun, would you mind having a look when you have some time?

kielnino avatar May 16 '25 06:05 kielnino

Hi @kielnino !

Thank you for the heads-up ! I went digging a bit on why it went un-noticed, and actually, it is because we always a bit of LOCAL bundle afterwards anyway, so it could potentially impact only the shot triangulation right after resection.

After playing on many datasets (Drone and Street-view), I couldn't any radical difference on the final results, so I've activated it anyway.

The branch is here and will be merged soon : https://github.com/mapillary/OpenSfM/tree/fix-shot-adjust

Thank you,

Yann

YanNoun avatar May 22 '25 08:05 YanNoun

Hi @YanNoun, thank you for coming back to this. I agree that the impact of the change is rather small, but at least it is now working as expected.

kielnino avatar May 22 '25 08:05 kielnino

Hi @YanNoun, do you have an idea when this will finally be merged?

kielnino avatar Jul 30 '25 07:07 kielnino