Add camera intrinsics priors and parameter bounds to bundle adjustment - #4683
Add camera intrinsics priors and parameter bounds to bundle adjustment#4683lpanaf wants to merge 7 commits into
Conversation
Bundle adjustment had no prior on the camera intrinsics: focal length, principal point and distortion parameters were free to drift arbitrarily, and the only guardrail was the post-hoc Camera::HasBogusParams filter that discards frames whose intrinsics have already degenerated. This adds two complementary mechanisms to BundleAdjustmentOptions: * Soft priors, enabled per parameter group by a weight. Focal length and principal point deviations are normalized by the maximum image dimension so the weights are resolution independent; extra parameters are already unitless. The focal length prior only applies to cameras with has_prior_focal_length, the principal point and extra parameter priors pull towards the default initialization values (image center and zero). All priors of a camera share one residual block with a Huber loss. * Hard caps (bound_camera_params, on by default except for local BA and the single-image absolute pose refinement) that register Ceres box constraints using exactly the bounds HasBogusParams tests, so the intrinsics cannot leave the region the filter considers valid. Parameters that already violate the bounds are clamped first, since Ceres rejects an infeasible starting point. Known limitation: there is no dedicated storage for the prior focal length (prior_focal_length in the database is a boolean flag), so the focal prior is snapshotted from camera.params when the problem is built. Across successive bundle adjustment problems it therefore acts as a damping term rather than an anchor to the original EXIF value. Pinning it properly would require a new Camera field and a schema migration.
The bounds and prior targets added in the previous commit assumed the generic perspective parameter layout, which does not hold for every camera model: * EUCMCameraModel::HasBogusExtraParams additionally requires alpha in [0, 1] and beta > 0, so the symmetric [-max_extra_param, max_extra_param] box admitted parameters that the bogus parameter filter rejects. * EUCM initializes beta to 1, so pulling all extra parameters towards zero dragged beta towards the degenerate beta <= 0 region. FOV initializes omega to 1e-2 rather than zero. Introduces a per-model ParamsBounds hook next to the HasBogus* checks, with a generic implementation for perspective models, an unbounded one for spherical models, an EUCM override, and a CameraModelParamsBounds dispatcher. Bundle adjustment now derives the bounds from it and takes the principal point and extra parameter prior targets from CameraModelInitializeParams, clamped into those bounds so a prior never pulls towards a rejected value. Also skips a parameter whose bound interval is empty, which is now reachable and would be undefined behavior in std::clamp. A parameterized test asserts for every camera model that clamping into the bounds yields parameters the bogus parameter filter accepts; it is built from the CAMERA_MODEL_CASES macro so future models are covered automatically. Verified that it fails without the EUCM override, and that the new EUCM bundle adjustment test fails with the previous prior targets. Audit of the other 16 models found no further inconsistencies: focal length and principal point bounds already matched the filter for all of them, all perspective models have exactly two principal point parameters defaulting to the image center, the remaining distortion parameters default to zero, and EQUIRECTANGULAR is excluded because it has no such parameters.
# Conflicts: # src/colmap/estimators/bundle_adjustment_ceres.cc
Focal length and principal point prior weights now directly multiply pixel deviations instead of being normalized by the maximum image dimension. The extra parameter weight is scaled by the mean focal length to convert dimensionless deviations into pixel-like residuals.
# Conflicts: # src/colmap/estimators/bundle_adjustment_ceres.cc
| // length. The principal point and | ||
| // extra parameter priors pull towards the values the camera model | ||
| // initializes them to, i.e. the image center and, for most models, zero. | ||
| double focal_length_prior_weight = 0; |
There was a problem hiding this comment.
How about use variance std as parameters and CovarianceWeighted wrapper where std = 1/weight
| // residuals = weights .* (params - priors), such that parameters with a zero | ||
| // weight are unconstrained. | ||
| template <typename CameraModel> | ||
| class CameraParamsPriorCostFunctor |
There was a problem hiding this comment.
Can we use the existing NormalPrior utility here: https://github.com/colmap/colmap/blob/main/src/colmap/estimators/cost_functions/utils.h#L73
ahojnnes
left a comment
ahojnnes
left a comment
There was a problem hiding this comment.
Thanks very much. Left some comments. Once we go through a first round of reviews, I suggest we also run some benchmark numbers.
| // Local bundle adjustment only optimizes a small subset of the problem, so | ||
| // the intrinsics are not bounded here but only during global refinement. | ||
| options.bound_camera_params = false; |
There was a problem hiding this comment.
Not sure I understand the motivation for this. If anything, I would expect that bounding the parameters in local BA is even more important than during global BA, because there are fewer constraints?
There was a problem hiding this comment.
The initial motivation is to still allow the model to reject bad results in the local stage, as we discussed before. We can change to enable it as well
| // residuals = weights .* (params - priors), such that parameters with a zero | ||
| // weight are unconstrained. | ||
| template <typename CameraModel> | ||
| class CameraParamsPriorCostFunctor |
There was a problem hiding this comment.
Maybe we generalize this to "SoftPriorCostFunctor" or so? There is nothing specific to cameras in this function and it could be reused for other purposes?
| VectorN weights_; | ||
| VectorN priors_; |
| // with Camera::HasBogusParams, such that the intrinsics cannot leave the | ||
| // region that the bogus parameter filter considers valid. Parameters that | ||
| // already violate the bounds are clamped into them. | ||
| bool bound_camera_params = true; |
There was a problem hiding this comment.
Maybe set this to false by default?
| if (upper == lower) { | ||
| // Ceres rejects an empty bound interval as infeasible. The clamp | ||
| // above already pins the parameter to the only feasible value. | ||
| return; | ||
| } |
There was a problem hiding this comment.
Can be combined with upper < lower as upper <= lower ?
| *problem_); | ||
|
|
||
| // Bound the camera parameters before adding the priors, so that the priors | ||
| // are anchored at feasible values. |
There was a problem hiding this comment.
| // are anchored at feasible values. | |
| // are anchored at feasible values. Notice that BoundCameraParams() clamps the input camera parameters to the valid range. |
|
|
||
| // A box constraint cannot express the strict beta > 0 that HasBogusExtraParams | ||
| // requires, so beta is bounded from below by this small positive value. | ||
| constexpr double kMinEUCMBeta = 1e-6; |
There was a problem hiding this comment.
Suggest moving this inside the ParamsBounds method.
| // initializes them to, i.e. the image center and, for most models, zero. | ||
| double focal_length_prior_weight = 0; | ||
| double principal_point_prior_weight = 0; | ||
| double extra_params_prior_weight = 0; |
There was a problem hiding this comment.
Going with "weight" is probably fine here. The alternative would be "stddev". Maybe add a biref note about how these weights relate to expected standard deviations of the priors?
| // direct multipliers on deviations in pixels. The extra parameter weight is | ||
| // multiplied by the mean focal length at the time the problem is constructed | ||
| // to convert dimensionless deviations to pixel-like residuals. |
There was a problem hiding this comment.
I am a bit torn here on whether to keep the extra weight just defined on the original scale vs. this rescaling approach vs. an alternative rescaling approach.
Small FOV cameras with equal resolution will have a higher weight on the extra parameters as compared to large FOV cameras. Wondering whether it instead makes more sense to rescale with max(width, height) ? cc: @B1ueber2y @sarlinpe
I just put the results based on the current pr to the description. |
For more complex models, the difference is huge. Am I interpreting this right? |
|
For the distorted/undistorted results, are these both with --calibrated or --uncalibrated? |
uncalibrated |
Yes, this is also what I see: for the undistorted images, the difference is pretty large for complex camera models, while for the distorted images, adding the prior hurts a bit, but not too much. |
|
Isn't it a bit surprising that the difference is much larger for undistorted than distorted? I would have expected the vice versa? |
This is not surprising to me: for the undistorted images, the extra parameters can be very much under-determined, while for the the distorted images, distortion are strong enough to give more reasonable values for the extra parameters. Additionally, for the undisorted cases, I tested OPENCV_FULL and EUCM, while I tested other models for distorted images. I will run these two models there as well to draw further conclusions. |
Replace CameraParamsPriorCostFunctor with the upstream ScaleWeightedCostFunctor composed with NormalPriorCostFunctor. Parameters without a prior get an infinite standard deviation, which the wrapper inverts to a zero multiplier and thus leaves unconstrained. Make the intrinsics bounds opt-in everywhere: default BundleAdjustmentOptions::bound_camera_params to false and drive both incremental bundle adjustment paths from the new, user-settable Mapper.ba_bound_camera_params instead of hardcoding them. Also simplify the degenerate bound interval handling, scope kMinBeta to EUCMCameraModel::ParamsBounds, and document that the prior weights are inverse standard deviations.
|
Added incremental_mapper results to the description |
3 participants
Adds soft priors on camera intrinsics (focal length, principal point, extra params) with camera-model-aware bounds and prior targets. Focal length and principal point weights are direct pixel-space multipliers; the extra parameter weight is scaled by the mean focal length to produce pixel-like residuals.
Results based on the current draft PR (
with
global_mapperUndistorted Images
All-25 Pooled Results
All-25 Mode Deltas
Distorted Images
The distorted panel evaluates the same 25 scenes and 898 images with
SIMPLE_RADIALandTHIN_PRISM_FISHEYE. Each all-25 row comes from one complete report; the original-13 and test-12 rows below are raw-error-pooled subsets of those reports.All-25 Pooled Results
All-25 Mode Deltas
With incremental mapper
Undistorted
Undistorted mode deltas
Where the regression comes from
Pooled over all 25 scenes / 898 images:
Seed sweep: AUC@10, bounds_off -> bounds_on
Seeds where
bounds_on >= bounds_offon AUC@10: lecture_room 3/4,old_computer 2/4, playground 2/4. Overall 7 of 12 seed-scene pairs
favour bounds on.
Distorted
Distorted mode deltas