Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Improve logging for errors in masking during feature extraction#3034

Merged
ahojnnes merged 13 commits into
colmap:maincolmap/colmap:mainfrom
Ambrosiussen:mainAmbrosiussen/colmap:mainCopy head branch name to clipboard
Dec 18, 2024
Merged

Improve logging for errors in masking during feature extraction#3034
ahojnnes merged 13 commits into
colmap:maincolmap/colmap:mainfrom
Ambrosiussen:mainAmbrosiussen/colmap:mainCopy head branch name to clipboard

Conversation

@Ambrosiussen

Copy link
Copy Markdown
Contributor

The Masking logging is quite unhelpful currently.
There is no indication if the mask arguments used in the Feature Extractor are correct, or if a mask for any given image is missing.
This PR helps visualizing this.

Comment thread src/colmap/controllers/feature_extraction.cc
Co-authored-by: Johannes Schönberger <jsch@demuc.de>
Comment thread src/colmap/controllers/image_reader.cc
Comment thread src/colmap/controllers/image_reader.cc Outdated
Ambrosiussen and others added 2 commits December 12, 2024 12:32

@ahojnnes ahojnnes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. LGTM.

@ahojnnes ahojnnes changed the title Improve Masking Logging Improve logging for errors in masking during feature extraction Dec 12, 2024
@ahojnnes
ahojnnes enabled auto-merge (squash) December 12, 2024 11:43
auto-merge was automatically disabled December 12, 2024 11:47

Head branch was pushed to by a user without write access

@ahojnnes

Copy link
Copy Markdown
Contributor

@Ambrosiussen

Copy link
Copy Markdown
Contributor Author

@Ambrosiussen There seems to be a bug in the logic, see: https://github.com/colmap/colmap/actions/runs/12295764100/job/34313358272?pr=3034

I think it actually exposed a bug! If you look at the dataset, and the arguments used for automatic reconstruction. No mask is supposed to be used at all!

Yet somehow it passes this check which was already there :)
if (mask && !options_.mask_path.empty()) {

ubprocess.CalledProcessError: Command '['/home/runner/work/colmap/colmap/build/src/colmap/exe/colmap', 'automatic_reconstructor', '--image_path', 'boulders/images/', '--workspace_path', '/home/runner/work/colmap/colmap/eth3d_benchmark/boulders', '--use_gpu', '1', '--num_threads', '-1', '--quality', 'low', '--camera_model', 'PINHOLE', '--camera_params', '3408.29,3408.61,3115.6,2067.54']'

@ahojnnes

Copy link
Copy Markdown
Contributor

image_reader.cc:56 needs the following I would guess:

  if (options_.mask_path) {
    options_.mask_path =
        EnsureTrailingSlash(StringReplace(options_.mask_path, "\\", "/"));
  }

@Ambrosiussen

Ambrosiussen commented Dec 18, 2024

Copy link
Copy Markdown
Contributor Author

@ahojnnes Any idea why these tests dont run? EDIT: And right after I comment this they magically start running :D

@ahojnnes

Copy link
Copy Markdown
Contributor

They need to be approved by a maintainer for first time contributors.

@ahojnnes
ahojnnes merged commit 0a6c915 into colmap:main Dec 18, 2024
@ahojnnes

Copy link
Copy Markdown
Contributor

Thank you for your contribution.

HernandoR pushed a commit to HernandoR/colmap that referenced this pull request Dec 30, 2024
…ap#3034)

The Masking logging is quite unhelpful currently.
There is no indication if the mask arguments used in the Feature
Extractor are correct, or if a mask for any given image is missing.
This PR helps visualizing this.

---------

Co-authored-by: Johannes Schönberger <jsch@demuc.de>
Co-authored-by: Paul-Edouard Sarlin <15985472+sarlinpe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.