Code cleanup (#29)

* Rename .cc files to .h

* Remove unnecessary forward declarations

* Move top include directory

* Use clang-format of COLMAP

* Add missing pragma once

* Format example notebooks as well

* Add CI

.clang-format

@@ -1,66 +1,13 @@
----
-Language:        Cpp
-# BasedOnStyle:  Google
-AccessModifierOffset: -1
-AlignAfterOpenBracket: true
-AlignConsecutiveAssignments: false
-AlignEscapedNewlinesLeft: true
-AlignOperands:   true
-AlignTrailingComments: true
-AllowAllParametersOfDeclarationOnNextLine: true
-AllowShortBlocksOnASingleLine: false
-AllowShortCaseLabelsOnASingleLine: false
-AllowShortFunctionsOnASingleLine: All
-AllowShortIfStatementsOnASingleLine: true
-AllowShortLoopsOnASingleLine: true
-AlwaysBreakAfterDefinitionReturnType: None
-AlwaysBreakBeforeMultilineStrings: true
-AlwaysBreakTemplateDeclarations: true
-BinPackArguments: true
-BinPackParameters: true
-BreakBeforeBinaryOperators: None
-BreakBeforeBraces: Attach
-BreakBeforeTernaryOperators: true
-BreakConstructorInitializersBeforeComma: false
-ColumnLimit:     90
-CommentPragmas:  '^ IWYU pragma:'
-ConstructorInitializerAllOnOneLineOrOnePerLine: true
-ConstructorInitializerIndentWidth: 4
-ContinuationIndentWidth: 4
-Cpp11BracedListStyle: true
+BasedOnStyle: Google
+BinPackArguments: false
+BinPackParameters: false
 DerivePointerAlignment: false
-DisableFormat:   false
-ExperimentalAutoDetectBinPacking: false
-ForEachMacros:   [ foreach, Q_FOREACH, BOOST_FOREACH ]
-IndentCaseLabels: true
-IndentWidth:     2
-IndentWrappedFunctionNames: false
-KeepEmptyLinesAtTheStartOfBlocks: false
-MacroBlockBegin: ''
-MacroBlockEnd:   ''
-MaxEmptyLinesToKeep: 1
-NamespaceIndentation: None
-ObjCBlockIndentWidth: 2
-ObjCSpaceAfterProperty: false
-ObjCSpaceBeforeProtocolList: false
-PenaltyBreakBeforeFirstCallParameter: 1
-PenaltyBreakComment: 300
-PenaltyBreakFirstLessLess: 120
-PenaltyBreakString: 1000
-PenaltyExcessCharacter: 1000000
-PenaltyReturnTypeOnItsOwnLine: 200
-PointerAlignment: Left
-SpaceAfterCStyleCast: false
-SpaceBeforeAssignmentOperators: true
-SpaceBeforeParens: ControlStatements
-SpaceInEmptyParentheses: false
-SpacesBeforeTrailingComments: 2
-SpacesInAngles:  false
-SpacesInContainerLiterals: true
-SpacesInCStyleCastParentheses: false
-SpacesInParentheses: false
-SpacesInSquareBrackets: false
-Standard:        Auto
-TabWidth:        8
-UseTab:          Never
-...
+IncludeBlocks: Regroup
+IncludeCategories:
+  - Regex:    '^"_pyceres'
+    Priority: 1
+  - Regex:    '^<[[:alnum:]_]+>'
+    Priority: 2
+  - Regex:    '.*'
+    Priority: 3
+SortIncludes: true

.github/workflows/code-formatting.yml

@@ -0,0 +1,25 @@
+name: clang-format Check
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+    types: [ assigned, opened, synchronize, reopened ]
+jobs:
+  formatting-check:
+    name: Formatting Check
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - name: Run clang-format style check for C/C++/Protobuf programs.
+      uses: jidicula/[email protected]
+      with:
+        clang-format-version: '14'
+    - uses: actions/setup-python@v4
+      with:
+        python-version: '3.10'
+        cache: 'pip'
+    - run: python -m pip install --upgrade pip
+    - run: python -m pip install isort black
+    - run: python -m isort . --profile black --check-only --diff
+    - run: python -m black . --check --diff

CMakeLists.txt

@@ -18,7 +18,7 @@ find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
 find_package(pybind11 2.11.1 REQUIRED)
 
 pybind11_add_module(pyceres _pyceres/bindings.cc)
-target_include_directories(pyceres PRIVATE ${PROJECT_SOURCE_DIR}/_pyceres)
+target_include_directories(pyceres PRIVATE ${PROJECT_SOURCE_DIR})
 target_link_libraries(pyceres PRIVATE colmap::colmap glog::glog Ceres::ceres)
 target_compile_definitions(pyceres PRIVATE VERSION_INFO="${PROJECT_VERSION}")
 install(TARGETS pyceres LIBRARY DESTINATION .)

_pyceres/bindings.cc

@@ -1,16 +1,15 @@
+#include "_pyceres/core/bindings.h"
+
+#include "_pyceres/factors/bindings.h"
+#include "_pyceres/glog.h"
+#include "_pyceres/helpers.h"
+
 #include <pybind11/iostream.h>
 #include <pybind11/pybind11.h>
 #include <pybind11/stl.h>
 #include <pybind11/stl_bind.h>
-namespace py = pybind11;
 
-#include "core/bindings.cc"
-#include "factors/bindings.cc"
-#include "glog.cc"
-#include "helpers.h"
-
-void bind_core(py::module&);
-void bind_factors(py::module&);
+namespace py = pybind11;
 
 PYBIND11_MODULE(pyceres, m) {
   m.doc() = "PyCeres";

_pyceres/core/bindings.cc → _pyceres/core/bindings.h

@@ -28,27 +28,20 @@
 // Author: [email protected] (Nikolaus Mitchell)
 // Edited by: [email protected] (Philipp Lindenberger)
 
-#include <pybind11/pybind11.h>
-namespace py = pybind11;
+#pragma once
 
-#include "core/callbacks.cc"
-#include "core/cost_functions.cc"
-#include "core/covariance.cc"
-#include "core/loss_functions.cc"
-#include "core/manifold.cc"
-#include "core/problem.cc"
-#include "core/solver.cc"
-#include "core/types.cc"
+#include "_pyceres/core/callbacks.h"
+#include "_pyceres/core/cost_functions.h"
+#include "_pyceres/core/covariance.h"
+#include "_pyceres/core/loss_functions.h"
+#include "_pyceres/core/manifold.h"
+#include "_pyceres/core/problem.h"
+#include "_pyceres/core/solver.h"
+#include "_pyceres/core/types.h"
 
-void init_types(py::module& m);
-void init_callbacks(py::module& m);
-void init_covariance(py::module& m);
-void init_solver(py::module& m);
-void init_loss_functions(py::module& m);
-void init_cost_functions(py::module& m);
-void init_manifold(py::module& m);
-void init_problem(py::module& m);
-void init_pyceres(py::module& m);
+#include <pybind11/pybind11.h>
+
+namespace py = pybind11;
 
 void bind_core(py::module& m) {
   init_types(m);

_pyceres/core/callbacks.cc → _pyceres/core/callbacks.h

@@ -27,12 +27,14 @@
 //
 // Author: [email protected] (Nikolaus Mitchell)
 // Edited by: [email protected] (Philipp Lindenberger)
+
+#pragma once
+
+#include <ceres/ceres.h>
 #include <pybind11/pybind11.h>
 #include <pybind11/stl.h>
 #include <pybind11/stl_bind.h>
 
-#include <ceres/ceres.h>
-
 namespace py = pybind11;
 
 // Trampoline class so we can create an EvaluationCallback in Python.
@@ -44,10 +46,11 @@ class PyEvaluationCallBack : public ceres::EvaluationCallback {
   void PrepareForEvaluation(bool evaluate_jacobians,
                             bool new_evaluation_point) override {
     PYBIND11_OVERLOAD_PURE(
-        void,                                    /* Return type */
-        ceres::EvaluationCallback,               /* Parent class */
-        PrepareForEvaluation,                    // Name of function in C++ (fn)
-        evaluate_jacobians, new_evaluation_point /* Argument(s) */
+        void,                      /* Return type */
+        ceres::EvaluationCallback, /* Parent class */
+        PrepareForEvaluation,      // Name of function in C++ (fn)
+        evaluate_jacobians,
+        new_evaluation_point /* Argument(s) */
     );
   }
 };
@@ -93,4 +96,4 @@ void init_callbacks(py::module& m) {
       py::keep_alive<1, 2>());
   py::implicitly_convertible<py::list,
                              std::vector<ceres::IterationCallback*>>();
-}
+}

_pyceres/core/cost_functions.cc → _pyceres/core/cost_functions.h

@@ -28,13 +28,15 @@
 // Author: [email protected] (Nikolaus Mitchell)
 // Edited by: [email protected] (Philipp Lindenberger)
 
-#include <pybind11/pybind11.h>
-namespace py = pybind11;
+#pragma once
+
+#include "_pyceres/helpers.h"
+#include "_pyceres/log_exceptions.h"
 
 #include <ceres/ceres.h>
+#include <pybind11/pybind11.h>
 
-#include "helpers.h"
-#include "log_exceptions.h"
+namespace py = pybind11;
 
 // Class which we can use to create a ceres::CostFunction in python.
 // This allows use to create python based cost functions.
@@ -44,7 +46,8 @@ class PyCostFunction : public ceres::CostFunction {
   using ceres::CostFunction::CostFunction;
   using ceres::CostFunction::set_num_residuals;
 
-  bool Evaluate(double const* const* parameters, double* residuals,
+  bool Evaluate(double const* const* parameters,
+                double* residuals,
                 double** jacobians) const override {
     pybind11::gil_scoped_acquire gil;
 
@@ -57,9 +60,10 @@ class PyCostFunction : public ceres::CostFunction {
       for (size_t idx = 0; idx < parameter_block_sizes().size(); ++idx) {
         parameters_vec.emplace_back(py::array_t<double>(
             this->parameter_block_sizes()[idx], parameters[idx], no_copy));
-        jacobians_vec.emplace_back(
-            py::array_t<double>(this->parameter_block_sizes()[idx] * num_residuals(),
-                                jacobians[idx], no_copy));
+        jacobians_vec.emplace_back(py::array_t<double>(
+            this->parameter_block_sizes()[idx] * num_residuals(),
+            jacobians[idx],
+            no_copy));
       }
       cached_flag = true;
     }
@@ -72,26 +76,28 @@ class PyCostFunction : public ceres::CostFunction {
     info = parameters_vec[0].request(true);
     if (info.ptr != parameters) {
       for (size_t idx = 0; idx < parameters_vec.size(); ++idx) {
-        parameters_vec[idx] = py::array_t<double>(this->parameter_block_sizes()[idx],
-                                                  parameters[idx], no_copy);
+        parameters_vec[idx] = py::array_t<double>(
+            this->parameter_block_sizes()[idx], parameters[idx], no_copy);
       }
     }
     if (jacobians) {
       info = jacobians_vec[0].request(true);
       if (info.ptr != jacobians) {
         for (size_t idx = 0; idx < jacobians_vec.size(); ++idx) {
-          jacobians_vec[idx] =
-              py::array_t<double>(this->parameter_block_sizes()[idx] * num_residuals(),
-                                  jacobians[idx], no_copy);
+          jacobians_vec[idx] = py::array_t<double>(
+              this->parameter_block_sizes()[idx] * num_residuals(),
+              jacobians[idx],
+              no_copy);
         }
       }
     }
 
-    pybind11::function overload =
-        pybind11::get_overload(static_cast<const ceres::CostFunction*>(this), "Evaluate");
+    pybind11::function overload = pybind11::get_overload(
+        static_cast<const ceres::CostFunction*>(this), "Evaluate");
     if (overload) {
       auto o = overload.operator()<pybind11::return_value_policy::reference>(
-          parameters_vec, residuals_wrap,
+          parameters_vec,
+          residuals_wrap,
           jacobians ? &jacobians_vec : nullptr);  // nullptr gets cast to None
       return pybind11::detail::cast_safe<bool>(std::move(o));
     }
@@ -108,21 +114,24 @@ class PyCostFunction : public ceres::CostFunction {
   mutable std::vector<py::array_t<double>> jacobians_vec;
   mutable bool cached_flag = false;  // Flag used to determine if the vectors
   // need to be resized
-  mutable py::array_t<double> residuals_wrap;  // Buffer to contain the residuals
+  mutable py::array_t<double>
+      residuals_wrap;  // Buffer to contain the residuals
   // pointer
   mutable py::str no_copy;  // Dummy variable for pybind11 to avoid copy
                             // copy
 };
 
 void init_cost_functions(py::module& m) {
-  py::class_<ceres::CostFunction, PyCostFunction /* <--- trampoline*/>(m, "CostFunction")
+  py::class_<ceres::CostFunction, PyCostFunction /* <--- trampoline*/>(
+      m, "CostFunction")
       .def(py::init<>())
       .def("num_residuals", &ceres::CostFunction::num_residuals)
       .def("num_parameter_blocks",
            [](ceres::CostFunction& myself) {
              return myself.parameter_block_sizes().size();
            })
-      .def("parameter_block_sizes", &ceres::CostFunction::parameter_block_sizes,
+      .def("parameter_block_sizes",
+           &ceres::CostFunction::parameter_block_sizes,
            py::return_value_policy::reference)
       .def("set_num_residuals", &PyCostFunction::set_num_residuals)
       .def("set_parameter_block_sizes",
@@ -135,31 +144,36 @@ void init_cost_functions(py::module& m) {
       .def(
           "evaluate",
           [](ceres::CostFunction& self, const py::args& parameters) {
-            THROW_CHECK_EQ(parameters.size(), self.parameter_block_sizes().size());
+            THROW_CHECK_EQ(parameters.size(),
+                           self.parameter_block_sizes().size());
             std::vector<double*> params(parameters.size());
-            Eigen::Matrix<double, -1, -1, Eigen::RowMajor> residuals(self.num_residuals(),
-                                                                     1);
-            std::vector<Eigen::Matrix<double, -1, -1, Eigen::RowMajor>> jacobians;
+            Eigen::Matrix<double, -1, -1, Eigen::RowMajor> residuals(
+                self.num_residuals(), 1);
+            std::vector<Eigen::Matrix<double, -1, -1, Eigen::RowMajor>>
+                jacobians;
             for (int i = 0; i < parameters.size(); i++) {
               py::buffer_info param_buf =
-                  parameters[i].cast<py::array_t<double, py::array::c_style>>().request();
+                  parameters[i]
+                      .cast<py::array_t<double, py::array::c_style>>()
+                      .request();
               params[i] = static_cast<double*>(param_buf.ptr);
               ssize_t num_dims = 1;
               std::vector<ssize_t> param_shape = param_buf.shape;
               for (int k = 0; k < param_shape.size(); k++) {
                 num_dims *= param_shape[k];
               }
               THROW_CHECK_EQ(num_dims, self.parameter_block_sizes()[i]);
-              jacobians.push_back(Eigen::Matrix<double, -1, -1, Eigen::RowMajor>(
-                  self.num_residuals(), num_dims));
+              jacobians.push_back(
+                  Eigen::Matrix<double, -1, -1, Eigen::RowMajor>(
+                      self.num_residuals(), num_dims));
             }
             std::vector<double*> jacobian_ptrs;
             for (int i = 0; i < parameters.size(); i++) {
               jacobian_ptrs.push_back(jacobians[i].data());
             }
 
-            bool success =
-                self.Evaluate(params.data(), residuals.data(), jacobian_ptrs.data());
+            bool success = self.Evaluate(
+                params.data(), residuals.data(), jacobian_ptrs.data());
             if (success) {
               return py::make_tuple(residuals, jacobians);
             } else {