From 43e6b11e5aab425603578ecd9f994e1b814a1de5 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Mon, 18 Sep 2023 17:57:15 +0200 Subject: [PATCH 1/6] Redirect glog to stderr --- main.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/main.cc b/main.cc index 964fcd7e..4f83de8b 100644 --- a/main.cc +++ b/main.cc @@ -27,6 +27,7 @@ using namespace pybind11::literals; #include "reconstruction/reconstruction.cc" #include "sift.cc" #include "utils.h" +#include void init_reconstruction(py::module&); void init_quaternion(py::module&); @@ -39,6 +40,10 @@ PYBIND11_MODULE(pycolmap, m) { m.attr("__version__") = py::str("dev"); #endif + google::InitGoogleLogging(""); + google::InstallFailureSignalHandler(); + FLAGS_logtostderr = true; + auto PyDevice = py::enum_(m, "Device") .value("auto", Device::AUTO) .value("cpu", Device::CPU) From 8788bfe77de78cf1752699c739b2ed267dd6188d Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Mon, 18 Sep 2023 23:31:03 +0200 Subject: [PATCH 2/6] Switch from cout to glog, remove verbose --- pipeline/extract_features.cc | 15 +------------ pipeline/images.cc | 32 ++++++---------------------- pipeline/match_features.cc | 20 ++---------------- pipeline/mvs.cc | 41 ++++++------------------------------ 4 files changed, 17 insertions(+), 91 deletions(-) diff --git a/pipeline/extract_features.cc b/pipeline/extract_features.cc index bab7a4f9..43ed0266 100644 --- a/pipeline/extract_features.cc +++ b/pipeline/extract_features.cc @@ -32,8 +32,7 @@ void extract_features(const py::object database_path_, const std::string camera_model, ImageReaderOptions reader_options, SiftExtractionOptions sift_options, - const Device device, - bool verbose) { + const Device device) { std::string database_path = py::str(database_path_).cast(); THROW_CHECK_MSG(!ExistsFile(database_path), database_path + " already exists."); @@ -61,22 +60,11 @@ void extract_features(const py::object database_path_, std::invalid_argument, "Invalid camera parameters."); - std::stringstream oss; - std::streambuf* oldcerr = nullptr; - std::streambuf* oldcout = nullptr; - if (!verbose) { - oldcout = std::cout.rdbuf(oss.rdbuf()); - } py::gil_scoped_release release; std::unique_ptr extractor = CreateFeatureExtractorController(reader_options, sift_options); - extractor->Start(); PyWait(extractor.get()); - - if (!verbose) { - std::cout.rdbuf(oldcout); - } } void init_extract_features(py::module& m) { @@ -170,6 +158,5 @@ void init_extract_features(py::module& m) { "reader_options"_a = ImageReaderOptions(), "sift_options"_a = sift_extraction_options, "device"_a = Device::AUTO, - "verbose"_a = true, "Extract SIFT Features and write them to database"); } diff --git a/pipeline/images.cc b/pipeline/images.cc index 954e3da7..e3a49a85 100644 --- a/pipeline/images.cc +++ b/pipeline/images.cc @@ -24,6 +24,7 @@ using namespace pybind11::literals; #include "helpers.h" #include "log_exceptions.h" +#include void import_images(const py::object database_path_, const py::object image_path_, @@ -121,8 +122,7 @@ void undistort_images(py::object output_path_, std::string output_type, CopyType copy_type, int num_patch_match_src_images, - UndistortCameraOptions undistort_camera_options, - bool verbose) { + UndistortCameraOptions undistort_camera_options) { std::string output_path = py::str(output_path_).cast(); std::string input_path = py::str(input_path_).cast(); THROW_CHECK_DIR_EXISTS(input_path); @@ -130,17 +130,11 @@ void undistort_images(py::object output_path_, THROW_CHECK_DIR_EXISTS(image_path); CreateDirIfNotExists(output_path); - if (verbose) { - PrintHeading1("Reading reconstruction"); - } Reconstruction reconstruction; reconstruction.Read(input_path); - if (verbose) { - std::cout << StringPrintf(" => Reconstruction with %d images and %d points", - reconstruction.NumImages(), - reconstruction.NumPoints3D()) - << std::endl; - } + LOG(INFO) << StringPrintf(" => Reconstruction with %d images and %d points", + reconstruction.NumImages(), + reconstruction.NumPoints3D()); std::vector image_ids; for (const auto& image_name : image_list) { @@ -148,10 +142,11 @@ void undistort_images(py::object output_path_, if (image != nullptr) { image_ids.push_back(image->ImageId()); } else { - std::cout << "WARN: Cannot find image " << image_name << std::endl; + LOG(WARNING) << "Cannot find image " << image_name; } } + py::gil_scoped_release release; std::unique_ptr undistorter; if (output_type == "COLMAP") { undistorter.reset(new COLMAPUndistorter(undistort_camera_options, @@ -173,20 +168,8 @@ void undistort_images(py::object output_path_, std::string("Invalid `output_type` - supported values are ") + "{'COLMAP', 'PMVS', 'CMP-MVS'}."); } - - std::stringstream oss; - std::streambuf* oldcout = nullptr; - if (!verbose) { - oldcout = std::cout.rdbuf(oss.rdbuf()); - } - - py::gil_scoped_release release; undistorter->Start(); PyWait(undistorter.get()); - - if (!verbose) { - std::cout.rdbuf(oldcout); - } } void init_images(py::module& m) { @@ -318,6 +301,5 @@ void init_images(py::module& m) { "copy_policy"_a = CopyType::COPY, "num_patch_match_src_images"_a = 20, "undistort_options"_a = undistort_options, - "verbose"_a = false, "Undistort images"); } diff --git a/pipeline/match_features.cc b/pipeline/match_features.cc index 09ff37eb..6347f782 100644 --- a/pipeline/match_features.cc +++ b/pipeline/match_features.cc @@ -33,8 +33,7 @@ void match_features(py::object database_path_, SiftMatchingOptions sift_options, const Opts& matching_options, const TwoViewGeometryOptions& verification_options, - const Device device, - bool verbose) { + const Device device) { const std::string database_path = py::str(database_path_).cast(); THROW_CHECK_FILE_EXISTS(database_path); try { @@ -51,20 +50,8 @@ void match_features(py::object database_path_, py::gil_scoped_release release; std::unique_ptr matcher = MatcherFactory( matching_options, sift_options, verification_options, database_path); - - std::stringstream oss; - std::streambuf* oldcerr = nullptr; - std::streambuf* oldcout = nullptr; - if (!verbose) { - oldcout = std::cout.rdbuf(oss.rdbuf()); - } - matcher->Start(); PyWait(matcher.get()); - - if (!verbose) { - std::cout.rdbuf(oldcout); - } } void verify_matches(const py::object database_path_, @@ -82,6 +69,7 @@ void verify_matches(const py::object database_path_, ImagePairsMatchingOptions matcher_options; matcher_options.match_list_path = pairs_path; + py::gil_scoped_release release; std::unique_ptr matcher = CreateImagePairsFeatureMatcher( matcher_options, sift_options, verification_options, database_path); matcher->Start(); @@ -245,7 +233,6 @@ void init_match_features(py::module& m) { "matching_options"_a = exhaustive_options, "verification_options"_a = verification_options, "device"_a = Device::AUTO, - "verbose"_a = true, "Exhaustive feature matching"); m.def("match_sequential", @@ -255,7 +242,6 @@ void init_match_features(py::module& m) { "matching_options"_a = sequential_options, "verification_options"_a = verification_options, "device"_a = Device::AUTO, - "verbose"_a = true, "Sequential feature matching"); m.def("match_spatial", @@ -265,7 +251,6 @@ void init_match_features(py::module& m) { "matching_options"_a = spatial_options, "verification_options"_a = verification_options, "device"_a = Device::AUTO, - "verbose"_a = true, "Spatial feature matching"); m.def("match_vocabtree", @@ -275,7 +260,6 @@ void init_match_features(py::module& m) { "matching_options"_a = vocabtree_options, "verification_options"_a = verification_options, "device"_a = Device::AUTO, - "verbose"_a = true, "Vocab tree feature matching"); m.def("verify_matches", diff --git a/pipeline/mvs.cc b/pipeline/mvs.cc index 7cf26fa7..d93738fc 100644 --- a/pipeline/mvs.cc +++ b/pipeline/mvs.cc @@ -29,13 +29,11 @@ void patch_match_stereo(py::object workspace_path_, std::string workspace_format, std::string pmvs_option_name, mvs::PatchMatchOptions options, - std::string config_path, - bool verbose) { + std::string config_path) { #ifndef COLMAP_CUDA_ENABLED - THROW_EXCEPTION( - std::runtime_error, - "ERROR: Dense stereo reconstruction requires CUDA, which is not " - "available on your system."); + THROW_EXCEPTION(std::runtime_error, + "Dense stereo reconstruction requires CUDA, which is not " + "available on the system."); return; #endif // COLMAP_CUDA_ENABLED std::string workspace_path = py::str(workspace_path_).cast(); @@ -48,22 +46,11 @@ void patch_match_stereo(py::object workspace_path_, "Invalid `workspace_format` - supported values are " "'COLMAP' or 'PMVS'.") - std::stringstream oss; - std::streambuf* oldcout = nullptr; - if (!verbose) { - oldcout = std::cout.rdbuf(oss.rdbuf()); - } - + py::gil_scoped_release release; mvs::PatchMatchController controller( options, workspace_path, workspace_format, pmvs_option_name, config_path); - controller.Start(); PyWait(&controller); - // controller.Wait(); - - if (!verbose) { - std::cout.rdbuf(oldcout); - } } Reconstruction stereo_fusion(py::object output_path_, @@ -71,8 +58,7 @@ Reconstruction stereo_fusion(py::object output_path_, std::string workspace_format, std::string pmvs_option_name, std::string input_type, - mvs::StereoFusionOptions options, - bool verbose) { + mvs::StereoFusionOptions options) { std::string workspace_path = py::str(workspace_path_).cast(); THROW_CHECK_DIR_EXISTS(workspace_path); @@ -92,24 +78,13 @@ Reconstruction stereo_fusion(py::object output_path_, "Invalid input type - supported values are 'photometric' and " "'geometric'."); + py::gil_scoped_release release; mvs::StereoFusion fuser( options, workspace_path, workspace_format, pmvs_option_name, input_type); - - std::stringstream oss; - std::streambuf* oldcout = nullptr; - if (!verbose) { - oldcout = std::cout.rdbuf(oss.rdbuf()); - } - py::gil_scoped_release release; fuser.Start(); PyWait(&fuser); - if (!verbose) { - std::cout.rdbuf(oldcout); - } - Reconstruction reconstruction; - // read data from sparse reconstruction if (workspace_format == "colmap") { reconstruction.Read(JoinPaths(workspace_path, "sparse")); @@ -282,7 +257,6 @@ void init_mvs(py::module& m) { "pmvs_option_name"_a = "option-all", "options"_a = patch_match_options, "config_path"_a = "", - "verbose"_a = true, "Runs Patch-Match-Stereo (requires CUDA)"); m.def("stereo_fusion", @@ -293,6 +267,5 @@ void init_mvs(py::module& m) { "pmvs_option_name"_a = "option-all", "input_type"_a = "geometric", "options"_a = stereo_fusion_options, - "verbose"_a = true, "Stereo Fusion"); } From fc72e98d0499373e57d3acf610f352ada80c2b8f Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Tue, 19 Sep 2023 00:01:52 +0200 Subject: [PATCH 3/6] Fix --- pipeline/match_features.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/pipeline/match_features.cc b/pipeline/match_features.cc index 6347f782..e5db46ec 100644 --- a/pipeline/match_features.cc +++ b/pipeline/match_features.cc @@ -69,7 +69,6 @@ void verify_matches(const py::object database_path_, ImagePairsMatchingOptions matcher_options; matcher_options.match_list_path = pairs_path; - py::gil_scoped_release release; std::unique_ptr matcher = CreateImagePairsFeatureMatcher( matcher_options, sift_options, verification_options, database_path); matcher->Start(); From 9e6ca1e168566e919d81b6e3df91443180233265 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Tue, 19 Sep 2023 13:16:48 +0200 Subject: [PATCH 4/6] Bind some glog attributes like in pyceres --- main.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/main.cc b/main.cc index 4f83de8b..74a8fd4a 100644 --- a/main.cc +++ b/main.cc @@ -32,6 +32,8 @@ using namespace pybind11::literals; void init_reconstruction(py::module&); void init_quaternion(py::module&); +class glog_dummy {}; // dummy class + PYBIND11_MODULE(pycolmap, m) { m.doc() = "COLMAP plugin"; #ifdef VERSION_INFO @@ -40,6 +42,12 @@ PYBIND11_MODULE(pycolmap, m) { m.attr("__version__") = py::str("dev"); #endif + py::class_(m, "glog") + .def_readwrite_static("minloglevel", &FLAGS_minloglevel) + .def_readwrite_static("stderrthreshold", &FLAGS_stderrthreshold) + .def_readwrite_static("log_dir", &FLAGS_log_dir) + .def_readwrite_static("logtostderr", &FLAGS_logtostderr) + .def_readwrite_static("alsologtostderr", &FLAGS_alsologtostderr); google::InitGoogleLogging(""); google::InstallFailureSignalHandler(); FLAGS_logtostderr = true; From 6872bd9f1f8292a5fcc7f6da8e9770b06f98ff76 Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Thu, 7 Dec 2023 18:46:11 +0100 Subject: [PATCH 5/6] Improve glog bindings --- main.cc | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/main.cc b/main.cc index 74a8fd4a..78d8daf2 100644 --- a/main.cc +++ b/main.cc @@ -32,7 +32,14 @@ using namespace pybind11::literals; void init_reconstruction(py::module&); void init_quaternion(py::module&); -class glog_dummy {}; // dummy class +struct Logging { + enum class Level { + INFO = google::GLOG_INFO, + WARNING = google::GLOG_WARNING, + ERROR = google::GLOG_ERROR, + FATAL = google::GLOG_FATAL, + }; +}; // dummy class PYBIND11_MODULE(pycolmap, m) { m.doc() = "COLMAP plugin"; @@ -42,12 +49,23 @@ PYBIND11_MODULE(pycolmap, m) { m.attr("__version__") = py::str("dev"); #endif - py::class_(m, "glog") - .def_readwrite_static("minloglevel", &FLAGS_minloglevel) - .def_readwrite_static("stderrthreshold", &FLAGS_stderrthreshold) - .def_readwrite_static("log_dir", &FLAGS_log_dir) - .def_readwrite_static("logtostderr", &FLAGS_logtostderr) - .def_readwrite_static("alsologtostderr", &FLAGS_alsologtostderr); + auto PyLogging = + py::class_(m, "logging") + .def_readwrite_static("minloglevel", &FLAGS_minloglevel) + .def_readwrite_static("stderrthreshold", &FLAGS_stderrthreshold) + .def_readwrite_static("log_dir", &FLAGS_log_dir) + .def_readwrite_static("logtostderr", &FLAGS_logtostderr) + .def_readwrite_static("alsologtostderr", &FLAGS_alsologtostderr) + .def_static("info", [](std::string msg) { LOG(INFO) << msg; }) + .def_static("warning", [](std::string msg) { LOG(WARNING) << msg; }) + .def_static("error", [](std::string msg) { LOG(ERROR) << msg; }) + .def_static("fatal", [](std::string msg) { LOG(FATAL) << msg; }); + py::enum_(PyLogging, "Level") + .value("INFO", Logging::Level::INFO) + .value("WARNING", Logging::Level::WARNING) + .value("ERROR", Logging::Level::ERROR) + .value("FATAL", Logging::Level::FATAL) + .export_values(); google::InitGoogleLogging(""); google::InstallFailureSignalHandler(); FLAGS_logtostderr = true; From e7368954e6fc584edc600c8a8501769bbcfab52c Mon Sep 17 00:00:00 2001 From: Paul-Edouard Sarlin Date: Fri, 8 Dec 2023 09:31:39 +0100 Subject: [PATCH 6/6] cerr -> LOG(ERROR) --- helpers.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/helpers.h b/helpers.h index 46930cab..4eda86b3 100644 --- a/helpers.h +++ b/helpers.h @@ -4,6 +4,7 @@ #include "colmap/util/threading.h" +#include #include #include #include @@ -26,7 +27,7 @@ inline T pyStringToEnum(const py::enum_& enm, const std::string& value) { return T(values[strVal].template cast()); } std::string msg = - "ERROR: Invalid string value " + value + " for enum " + + "Invalid string value " + value + " for enum " + std::string(enm.attr("__name__").template cast()); THROW_EXCEPTION(std::out_of_range, msg.c_str()); T t; @@ -91,7 +92,7 @@ inline void make_dataclass(py::class_ cls) { << "'."; // We write the err message to give info even if exceptions // is catched outside, e.g. in function overload resolve - std::cerr << "Internal TypeError: " << ss.str() << std::endl; + LOG(ERROR) << "Internal TypeError: " << ss.str(); throw( py::type_error(std::string("Failed to merge dict into class: ") + "Could not assign " + @@ -105,12 +106,12 @@ inline void make_dataclass(py::class_ cls) { << " defined readonly."; throw py::attribute_error(ss.str()); } else if (ex.matches(PyExc_AttributeError)) { - std::cerr << "Internal AttributeError: " - << py::str(ex.value()).cast() << std::endl; + LOG(ERROR) << "Internal AttributeError: " + << py::str(ex.value()).cast(); throw; } else { - std::cerr << "Internal Error: " - << py::str(ex.value()).cast() << std::endl; + LOG(ERROR) << "Internal Error: " + << py::str(ex.value()).cast(); throw; } } @@ -245,7 +246,7 @@ void PyWait(Thread* thread, double gap = 2.0) { PyInterrupt py_interrupt(gap); while (thread->IsRunning()) { if (py_interrupt.Raised()) { - std::cerr << "Stopping thread..." << std::endl; + LOG(ERROR) << "Stopping thread..."; thread->Stop(); thread->Wait(); throw py::error_already_set();