Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FeatureMatching: Prefix output matches files when using "ranges" to avoid overwrites #628

Merged
merged 7 commits into from
Jun 7, 2019

Conversation

yann-lty
Copy link
Member

@yann-lty yann-lty commented Apr 25, 2019

Description

Introduce an automatic prefix system for matches files when using "range" command-line parameters. The goal of this PR is to ensure a file created for one iteration does not get overwritten in another one.

When generating matches files per image, we rely on minimal viewId of each image pair to create the filename (minViewId.txt). By iterating over sorted viewIds which only match with greater viewIds, this guarantees that files are written only once.
However, the latter condition can be broken in the current pipeline when the file generated by the ImageMatching step declares for one view [B] a match with a view [A] that has an inferior viewId. This occurs when [A] has reached max view matches; the [A,B] view match is moved to [B]. If [A] and [B] are handled in two distinct iterations (1) and (2), "A.txt" will be first written in (1) and overwritten in (2). All matches from (1) are then lost.

By automatically prefixing matches files based on "range" parameters and generating only one matches file per execution by default, this guarantees uniqueness of generated files.
This also changes the loading of matches files, which will now consider all "*matches.txt" files in the given folder. This behavior is retro-compatible with previously generated files.

Features list

  • FeatureMatching: Generate a prefix for matches files based on range parameter
  • FeatureMatching: Generate only one matches file per execution by default
  • IO: Load all '*.matches.txt" file in given folder and accumulate matches
  • IO: Update IndMatch_IO test
  • IO: Remove duplicated input folders when loading matches/features
  • SfmData: manage features/matches folders internally (relative paths, duplicates removal...)

Implementation remarks

Prefix is created by dividing rangeStart/rangeSize which reflects the iteration number when working by chunks. This is preferred to random naming for debug purposes.

When using range parameters from main_featureMatching, prefix the resulting files with rangeStart/rangeSize  (i.e: iteration index when processing all views by chunks). 
=> with matchFilePerImage: avoids overwriting files if a view is present in several iterations
=> without matchFilePerImage: avoids overwriting the unique resulting file
* io: consider all files containing "matches.txt" when loading matches files from a folder
`matchFilePerImage` was activated by default to handle parallel runs of `featureMatching` on different ranges of views of the same scene. This is now handled by the prefix added to output files when using range related parameters.
* matches are accumulated when successively loading several files using the same PairwiseMatches variable without clearing it
* check for duplicates and test deduplication function
Ensure a folder is not considered twice when loading features and matches.
Copy link
Member

@simogasp simogasp left a comment

Choose a reason for hiding this comment

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

kudos for adding unit tests! 👍

src/aliceVision/matching/io.cpp Outdated Show resolved Hide resolved
src/aliceVision/matching/io.cpp Outdated Show resolved Hide resolved
src/aliceVision/matching/io.cpp Outdated Show resolved Hide resolved
src/aliceVision/sfmData/SfMData.cpp Outdated Show resolved Hide resolved
* handle conversions to relative and absolute paths when _absolutePath is defined
* update internal relative folder paths when absolutePath is modified
* ensure features/matches folders contains no duplicates
* [tests] add 'sfmData_test' module + test internal folders management
* [software] update global/incrementalSfm
@fabiencastan
Copy link
Member

fabiencastan commented Jun 4, 2019

We should update the major version of main_featureMatching.cpp (and remember to update it in Meshroom).

@fabiencastan fabiencastan added this to the 2019.2 milestone Jun 5, 2019
…vention has changed

This new version is able to load files with the previous naming convention but not the opposite.
BOOST_CHECK(fs::path(featuresFolders[0]).is_absolute());
BOOST_CHECK(fs::equivalent(featuresFolders[0], refFolder));
BOOST_CHECK(fs::path(matchesFolders[0]).is_absolute());
BOOST_CHECK(fs::equivalent(matchesFolders[0], refFolder));
Copy link
Member

Choose a reason for hiding this comment

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

We could try to add a new folder, like:

sfmData.setAbsolutePath(fs::absolute(filename).string());
sfmData.addFeaturesFolder(fs::absolute(filename) / "uselessFolder/..").string()); // duplicates from non-canonical path

to check the canonical path conversion with duplicates from different input strings.

@fabiencastan fabiencastan merged commit 132ac39 into develop Jun 7, 2019
@fabiencastan fabiencastan deleted the dev_matchingIO branch June 7, 2019 10:24
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