Use Matrix initializer_list constructor instead of comma initializers#2591
Use Matrix initializer_list constructor instead of comma initializers#2591Gold856 wants to merge 1 commit into
Conversation
dd86a0d to
7f3ebab
Compare
|
Awesome. But dynamic vs static should be changed deliberately. For Jacobians in particular, we don't want to incur extra overhead. That depends on the way the Jacobian arguments are given; some already are static and so we should not have a dynamic malloc there. |
There was a problem hiding this comment.
Pull request overview
This PR modernizes GTSAM’s examples, timing benchmarks, library code, and tests by replacing Eigen comma-initializers (<< ... .finished()) with brace/initializer_list-style construction for Matrix/Vector types.
Changes:
- Replaced Eigen comma-initializers with brace/initializer-list constructors across timing code, examples, and tests.
- Updated various fixed-size Eigen types (
Vector6,Matrix3, etc.) to use direct brace initialization. - Cleaned up a few small formatting/include details alongside the initializer migration.
Reviewed changes
Copilot reviewed 201 out of 201 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| timing/timeRot3.cpp | Replace Vector comma-init with braces |
| timing/timePose3.cpp | Replace Vector/Expmap init with braces |
| timing/timePose2.cpp | Replace Matrix/Vector comma-init with braces |
| timing/timeMatrix.cpp | Replace Matrix comma-init with braces |
| timing/timeLago.cpp | Replace noise-model sigma Vector init |
| timing/timeISAM2Covariance.cpp | Replace prior sigma Vector init |
| timing/timeGaussianFactor.cpp | Replace Matrix comma-init with braces |
| timing/timeCholesky.cpp | Replace Matrix comma-init with braces |
| timing/timeBayesTreeCovariance.cpp | Replace prior sigma Vector init |
| timing/timeActiveSetSolverComparison.cpp | Replace 1D Matrix/Vector constructors + brace init |
| timing/exportBayesTreeCovarianceVisuals.cpp | Replace prior sigma Vector init |
| tests/testPreconditioner.cpp | Replace Matrix/Vector comma-init with braces |
| tests/testIterative.cpp | Replace expected Vector init with braces |
| tests/testGaussianISAM2.cpp | Replace sigma Vector init with braces |
| tests/testGaussianFactorGraphB.cpp | Replace Matrix/Vector comma-init with braces |
| tests/testExtendedKalmanFilter.cpp | Replace Vector(1) init with braces |
| tests/testExpressionFactor.cpp | Replace expected Jacobian matrix init |
| tests/testBoundingConstraint.cpp | Replace small Jacobians and Vector(1) init |
| tests/smallExample.h | Replace small Matrix init with braces |
| gtsam/slam/tests/testStereoFactor.cpp | Replace expected Jacobian matrices init |
| gtsam/slam/tests/testSmartProjectionFactor.cpp | Replace VectorValues init with braces |
| gtsam/slam/tests/testRegularImplicitSchurFactor.cpp | Replace RHS Vector init with braces |
| gtsam/slam/tests/testProjectionFactor.cpp | Replace expected Jacobian matrices init |
| gtsam/slam/tests/testPriorFactor.cpp | Replace Vector6 comma-init with braces |
| gtsam/slam/tests/testPoseRotationPrior.cpp | Replace Vector(1) noise/error init |
| gtsam/slam/tests/testOrientedPlane3Factor.cpp | Replace sigma Vector init; formatting tweak needed |
| gtsam/slam/tests/testKarcherMeanFactor.cpp | Replace Vector6 init with braces |
| gtsam/slam/tests/testInitializePose3.cpp | Replace Vector/Matrix init with braces |
| gtsam/slam/tests/testGeneralSFMFactor.cpp | Replace landmark vector init style |
| gtsam/slam/tests/testGeneralSFMFactor_Cal3Bundler.cpp | Replace delta Vector init (currently broken) |
| gtsam/slam/tests/testFrobeniusFactor.cpp | Replace fixed-size vector/matrix init |
| gtsam/slam/tests/testEssentialMatrixFactor.cpp | Replace retract delta Vector init |
| gtsam/slam/RotateFactor.h | Replace returned Vector(3) construction |
| gtsam/slam/lago.cpp | Replace Vector(1)/Vector(2) init with braces |
| gtsam/slam/InitializePose3.cpp | Replace Vector(9) init with braces |
| gtsam/slam/dataset.cpp | Replace noise sigma Vector init |
| gtsam/slam/BoundingConstraint.h | Replace Vector(1) construction with braces |
| gtsam/sfm/tests/testTranslationFactor.cpp | Replace expected Vector3 init with braces |
| gtsam/sfm/tests/testShonanFactor.cpp | Replace fixed-size vector init |
| gtsam/sfm/tests/testSelfCalibrationFactor.cpp | Replace Vector(2) expected init |
| gtsam/sfm/SelfCalibrationFactor.h | Replace returned Vector(2) construction |
| gtsam/sam/tests/testRangeFactor.cpp | Replace Vector(1) expected init |
| gtsam/nonlinear/WhiteNoiseFactor.h | Replace scalar Matrix/Vector init with braces |
| gtsam/nonlinear/utilities.h | Replace Vector6 noise sigma init |
| gtsam/nonlinear/tests/testUtilities.cpp | Replace stored Vector init + expected Matrix init |
| gtsam/nonlinear/tests/testExtendedPriorFactor.cpp | Replace mean/expected_error Vector init |
| gtsam/nonlinear/tests/testBatchFixedLagSmoother.cpp | Replace sigma Vector init |
| gtsam/nonlinear/tests/testAdaptAutoDiff.cpp | Replace Vector9 init with braces |
| gtsam/nonlinear/NoiseModelFactorN.h | Update doc example Matrix init |
| gtsam/nonlinear/ISAM2Params.h | Update doc example Vector init |
| gtsam/navigation/tests/testNavStateImuEKF.cpp | Replace Matrix3 covariance init |
| gtsam/navigation/tests/testNavState.cpp | Replace Vector9 init with braces |
| gtsam/navigation/tests/testLieGroupEKF.cpp | Replace Matrix/Vector init with braces |
| gtsam/navigation/tests/testLeggedEstimatorFactors.cpp | Replace Matrix init with braces |
| gtsam/navigation/tests/testLeggedEstimator.cpp | Replace Matrix init with braces |
| gtsam/navigation/tests/testInvariantEKF.cpp | Replace Matrix/Vector init with braces |
| gtsam/navigation/tests/testImuFactor.cpp | Replace prior sigma vector init |
| gtsam/navigation/tests/testGal3ImuEKF.cpp | Replace Matrix3 covariance init |
| gtsam/navigation/tests/testEquivariantFilter.cpp | Replace Vector2 init with braces |
| gtsam/navigation/tests/testConstantVelocityFactor.cpp | Replace Vector9 expected init |
| gtsam/navigation/tests/testBarometricFactor.cpp | Replace Vector(1) init with braces |
| gtsam/navigation/NavState.h | Replace internal Matrix32 construction |
| gtsam/navigation/CarrierPhaseFactor.cpp | Replace scalar Matrix(1,1) init |
| gtsam/navigation/BarometricFactor.cpp | Replace Vector(1)/Matrix(1,1) init |
| gtsam/linear/VectorValues.h | Update example Vector init |
| gtsam/linear/tests/testVectorValues.cpp | Replace Vector init throughout tests |
| gtsam/linear/tests/testSparseEigen.cpp | Replace small Matrix init with braces |
| gtsam/linear/tests/testScatter.cpp | Use Matrix3 fixed-size init + constants |
| gtsam/linear/tests/testRegularJacobianFactor.cpp | Replace RHS/noise/init vectors |
| gtsam/linear/tests/testPowerMethod.cpp | Replace initial Vector init |
| gtsam/linear/tests/testGaussianDensity.cpp | Replace Matrix init with braces |
| gtsam/linear/tests/testBatchJacobianFactor.cpp | Replace A/B/b matrix/vector init |
| gtsam/linear/tests/testAcceleratedPowerMethod.cpp | Replace initial Vector init |
| gtsam/geometry/tests/testStereoCamera.cpp | Replace Matrix3 init in Pose3 |
| gtsam/geometry/tests/testSOn.cpp | Replace Vector6 init with braces |
| gtsam/geometry/tests/testSO4.cpp | Replace Vector6 init with braces |
| gtsam/geometry/tests/testSO3.cpp | Replace Matrix3/Matrix init with braces |
| gtsam/geometry/tests/testSimilarity3.cpp | Replace sigma Vector init |
| gtsam/geometry/tests/testSerializationGeometry.cpp | Replace Vector15 init |
| gtsam/geometry/tests/testRot2.cpp | Replace Vector1/Vector expected init |
| gtsam/geometry/tests/testPoint2.cpp | Replace Matrix(1,2) init |
| gtsam/geometry/tests/testExtendedPose3.cpp | Replace Matrix3/Vector12 init |
| gtsam/geometry/tests/testEssentialMatrix.cpp | Replace retract delta Vector init |
| gtsam/geometry/tests/testCal3DS2.cpp | Replace Vector(3) init |
| gtsam/geometry/SL4.cpp | Replace temp matrices/vectors with brace init; minor formatting |
| gtsam/geometry/Rot3Q.cpp | Replace Matrix3 comma-init with braces |
| gtsam/geometry/Pose3.cpp | Replace static Matrix63 init |
| gtsam/geometry/Event.h | Replace static Matrix14 init |
| gtsam/geometry/Cal3Unified.cpp | Replace Vector(1) print construction |
| gtsam/geometry/Cal3Bundler.cpp | Replace Vector(5) print construction |
| gtsam/constrained/tests/testQpsParser.cpp | Replace 1D Matrix/Vector init |
| gtsam/constrained/tests/testNonlinearEqualityConstraint.cpp | Replace expected vectors init |
| gtsam/constrained/tests/testLpProblem.cpp | Replace Values Vector + JacobianFactor init |
| gtsam/constrained/tests/testAugmentedLagrangianOptimizer.cpp | Replace lambdaEq Vector init |
| gtsam/constrained/QpsParser.cpp | Replace scalar Matrix/Vector init |
| gtsam/basis/tests/testFourier.cpp | Replace coefficient vectors init |
| gtsam/basis/tests/testChebyshev2.cpp | Replace Weights/Vector6 init |
| gtsam/base/tests/testVerticalBlockMatrix.cpp | Replace Matrix6 init |
| gtsam/base/tests/testVector.cpp | Remove comma-init test; should add initializer-list test |
| gtsam/base/tests/testSerializationBase.cpp | Replace matrix/vector init in serialization tests |
| gtsam/base/tests/testOptionalJacobian.cpp | Replace Matrix23 init |
| gtsam/base/tests/testCholesky.cpp | Replace matrices init with braces |
| gtsam/base/Matrix.h | Replace skewSymmetric Matrix3 init |
| gtsam_unstable/slam/TransformBtwRobotsUnaryFactorEM.h | Replace returned Vector(2) init |
| gtsam_unstable/slam/tests/testSmartRangeFactor.cpp | Replace Vector(1)/Matrix init |
| gtsam_unstable/slam/tests/testRelativeElevationFactor.cpp | Replace Vector(1) expected init |
| gtsam_unstable/slam/tests/testProjectionFactorPPPC.cpp | Replace expected Jacobian matrices init |
| gtsam_unstable/slam/tests/testProjectionFactorPPP.cpp | Replace expected Jacobian matrices init |
| gtsam_unstable/slam/tests/testPartialPriorFactor.cpp | Replace measurement Vector init |
| gtsam_unstable/slam/tests/testMultiProjectionFactor.cpp | Update commented expected Jacobian init |
| gtsam_unstable/slam/tests/testInvDepthFactorVariant3.cpp | Replace Vector3 expected + retract deltas |
| gtsam_unstable/slam/tests/testInvDepthFactorVariant2.cpp | Replace Vector3 expected + retract deltas |
| gtsam_unstable/slam/tests/testInvDepthFactorVariant1.cpp | Replace Vector6 expected + retract deltas |
| gtsam_unstable/slam/tests/testInvDepthFactor3.cpp | Replace Vector5 init |
| gtsam_unstable/slam/tests/testInertialNavFactor_GlobalVelocity.cpp | Replace measurement gyro vector init |
| gtsam_unstable/slam/tests/testAHRS.cpp | Replace matrices init with braces |
| gtsam_unstable/slam/RelativeElevationFactor.cpp | Replace Vector(1) return init |
| gtsam_unstable/slam/PartialPriorFactor.h | Replace prior_ Vector(1) init |
| gtsam_unstable/slam/Mechanization_bRn2.cpp | Replace Vector3 init with braces |
| gtsam_unstable/slam/InvDepthFactorVariant3.h | Replace Vector(1) return init |
| gtsam_unstable/slam/InvDepthFactorVariant2.h | Replace Vector(1) return init |
| gtsam_unstable/slam/InvDepthFactorVariant1.h | Replace Vector(1) return init |
| gtsam_unstable/slam/InvDepthFactor3.h | Replace Vector(1) return init |
| gtsam_unstable/slam/InertialNavFactor_GlobalVelocity.h | Replace ENU/NED matrices + vectors init |
| gtsam_unstable/slam/EquivInertialNavFactor_GlobalVel.h | Replace ENU/NED matrices + vectors init |
| gtsam_unstable/slam/EquivInertialNavFactor_GlobalVel_NoBias.h | Replace ENU/NED matrices init |
| gtsam_unstable/slam/BetweenFactorEM.h | Replace returned Vector(2) init |
| gtsam_unstable/slam/AHRS.cpp | Replace gravity vector + matrices init |
| gtsam_unstable/nonlinear/tests/testParticleFactor.cpp | Replace State(Vector) init |
| gtsam_unstable/nonlinear/tests/testConcurrentIncrementalSmootherGN.cpp | Replace noise sigma vector init |
| gtsam_unstable/nonlinear/tests/testConcurrentIncrementalSmootherDL.cpp | Replace noise sigma vector init |
| gtsam_unstable/nonlinear/tests/testConcurrentIncrementalFilter.cpp | Replace noise sigma vector init |
| gtsam_unstable/nonlinear/tests/testConcurrentBatchSmoother.cpp | Replace noise sigma vector init |
| gtsam_unstable/nonlinear/tests/testConcurrentBatchFilter.cpp | Replace noise sigma vector init |
| gtsam_unstable/navigation/tests/testEqVIOSymmetry.cpp | Replace SO3 expmap vectors init |
| gtsam_unstable/navigation/tests/testEqVIOGroup.cpp | Replace matrices/vectors init |
| gtsam_unstable/navigation/EqVIOSymmetry.cpp | Replace static Matrix32 init |
| gtsam_unstable/linear/tests/testQPSolver.cpp | Replace VectorValues/Matrix init |
| gtsam_unstable/linear/tests/testLinearEquality.cpp | Replace HessianFactor/VectorValues init |
| gtsam_unstable/linear/LinearInequality.h | Replace Vector(1) rhs init + formatting |
| gtsam_unstable/geometry/tests/testPose3Upright.cpp | Replace Vector delta init |
| gtsam_unstable/geometry/tests/testInvDepthCamera3.cpp | Replace Vector5/Vector init |
| gtsam_unstable/geometry/SimWall2D.cpp | Replace retract delta Vector init |
| gtsam_unstable/geometry/SimPolygon2D.cpp | Replace retract delta Vector init |
| gtsam_unstable/geometry/InvDepthCamera3.h | Replace Jacobian block Matrix init |
| gtsam_unstable/geometry/BearingS2.cpp | Replace Matrix init + add Vector include |
| gtsam_unstable/examples/ConcurrentCalibration.cpp | Replace sigma Vector init |
| gtsam_unstable/dynamics/VelocityConstraint3.h | Replace Vector(1) return init |
| gtsam_unstable/dynamics/tests/testSimpleHelicopter.cpp | Replace matrices/vectors init |
| gtsam_unstable/dynamics/tests/testPoseRTV.cpp | Replace vec_init Vector init |
| gtsam_unstable/dynamics/SimpleHelicopter.h | Replace f_ext Vector init |
| gtsam_unstable/dynamics/PoseRTV.cpp | Replace retract/rot_rates/drag vector init |
| gtsam_unstable/dynamics/Pendulum.h | Replace Vector(1) return init |
| gtsam_unstable/base/tests/testFixedVector.cpp | Replace Vector init |
| examples/SelfCalibrationExample.cpp | Replace calNoise sigma Vector init |
| examples/Pose3SLAMExampleExpressions_BearingRangeWithTransform.cpp | Replace noise sigma vectors init |
| examples/Pose3SLAMExample_initializePose3Gradient.cpp | Replace prior variances vector init |
| examples/Pose3SLAMExample_initializePose3Chordal.cpp | Replace prior variances vector init |
| examples/Pose3SLAMExample_g2o.cpp | Replace prior variances vector init |
| examples/Pose3Localization.cpp | Replace prior variances vector init |
| examples/Pose2SLAMExample_graph.cpp | Replace noise sigma Vector init |
| examples/LocalizationExample.cpp | Replace Jacobian Matrix + error Vector init |
| examples/LeggedEstimatorReplayExample.cpp | Replace covariance diagonal Vector init |
| examples/IMUKittiExampleGPS.cpp | Replace BodyP Vector6 init |
| examples/ImuFactorsExample.cpp | Replace pose noise sigma Vector init |
| examples/CombinedImuFactorsExample.cpp | Replace pose noise sigma Vector init |
| examples/GaussianProcessWnoaSE3Example.cpp | Replace Eigen vector init for diagonal/cmd |
| examples/elaboratePoint2KalmanFilter.cpp | Replace Vector2 sigma init |
| doc/Code/LocalizationFactor.cpp | Replace Matrix23 + error Vector init |
| Vector delta{{rot_noise, rot_noise, rot_noise}, // rotation | ||
| {trans_noise, trans_noise, trans_noise}, // translation | ||
| {focal_noise, distort_noise, distort_noise}}; // f, k1, k2 |
| /* ************************************************************************* */ | ||
| TEST(Vector, copy ) |
| // Init pose and prior. Pose Prior is needed since a single plane measurement | ||
| does not fully constrain the pose Pose3 init_pose(Rot3::Ypr(0.0, 0.0, 0.0), | ||
| Point3(0.0, 0.0, 0.0)); graph.addPrior(X(0), init_pose, | ||
| noiseModel::Diagonal::Sigmas( | ||
| (Vector(6) << 0.001, 0.001, 0.001, 0.001, 0.001, 0.001).finished())); | ||
| Vector{{0.001, 0.001, 0.001, 0.001, 0.001, 0.001}})); |
| #include <gtsam_unstable/geometry/BearingS2.h> | ||
|
|
||
| #include <cassert> | ||
| #include "gtsam/base/Vector.h" |
|
Also reviewed locally with Luna; findings:
The brace initializer itself is neutral: dynamic Eigen types allocate once, while fixed types store coefficients inline. The important policy is type selection and avoiding fixed/dynamic conversions at API boundaries. We could add a short “Eigen fixed vs. dynamic storage” section to
|
As promised :) One thing I wasn't exactly sure what to do was if I should use fixed size matrix types, or continue using the dynamically sized matrix types. I mostly opted for the dynamically sized matrix types, but I suspect fixed size would be better for ensuring correct initialization of matrices.
Also, happy 4th of July! 🎆