BundleShotPoses possibly broken
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.
@fabianschenk , could you take a look at this when you have time?
Steps to reproduce:
- Add the line
LOG(INFO) << ba.FullReport();at 564 - run
bin/opensfm_run_all data/berlin
@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.
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.
@YanNoun, would you mind having a look when you have some time?
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
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.
Hi @YanNoun, do you have an idea when this will finally be merged?