Skip to content
Snippets Groups Projects

Refactor all keypoints tests into a single test

Merged Julien Michel requested to merge refactor-keypoints-tests into develop

Summary

This MR refactors all keypoints tests into a single, more stable test.

Rationale

For years we endured feTvImageToFastSIFTKeyPointSetFilterSceneOutputInterestPointAscii and the like flickering our dasbhoard. They are now gone and testing strategy is stable and easier to adapt to different behavior of compilers / plateform.

Implementation Details

I removed all tests (and test code) related to Sift, SiftFast and Surf (as well as to the matching filter), and wrote a single test that generates a slightly rotated and scaled image from a reference image, and then for each aglorithm in Sift, SiftFast, and Surf do:

  1. Detect keypoints on both images
  2. Match keypoints
  3. Compute statistics on matching performance
  4. Use quality thresholds to decide if test passes or not.

Here is the ouptut of the new test:

 test 559
    Start 559: feTvKeyPointsAlgorithmsTest

559: Test command: /home/mp/michelj/scratch/dev/clang/otb/bin/otbDescriptorsTestDriver "otbKeyPointsAlgorithmsTest" "/work/scratch/michelj/dev/otb-data/Input/QB_TOULOUSE_RpcTag_100_100.tif"
559: Test timeout computed to be: 1500
559: 2019-01-11 16:26:48 (INFO): Default RAM limit for OTB is 128 MB
559: 2019-01-11 16:26:48 (INFO): GDAL maximum cache size is 3208 MB
559: 2019-01-11 16:26:48 (INFO): OTB will use at most 16 threads
559: 2019-01-11 16:26:49 (INFO): Loading kwl metadata from official product in file /work/scratch/michelj/dev/otb-data/Input/QB_TOULOUSE_RpcTag_100_100.tif
559: Secondary image generated by applying a rotation of 2.5 degrees and scaling of 0.989999999999999991118215802999.
559: Checking Surf implementation:
559: =============================
559: Found 405 points in reference image and 411 points in secondary image
559: Found 80 matches with 55 valid matches (tolerance of 0.5 pixels)
559: More than 15% of reference points have a match:    Ok (19.753086090087890625%)
559: More than 15% of secondary points have a match:    Ok (19.4647216796875%)
559: More than 65% of matches are good:                 Ok (68.75% at 0.5 pixel accuracy)
559: Checking Sift implementation:
559: =============================
559: Found 510 points in reference image and 486 points in secondary image
559: Found 241 matches with 217 valid matches (tolerance of 0.5 pixels)
559: More than 45% of reference points have a match:    Ok (47.254901885986328125%)
559: More than 45% of secondary points have a match:    Ok (49.58847808837890625%)
559: More than 85% of matches are good:                 Ok (90.04149627685546875% at 0.5 pixel accuracy)
559: Checking SiftFast implementation:
559: =================================
559: Found 441 points in reference image and 465 points in secondary image
559: Found 317 matches with 316 valid matches (tolerance of 0.5 pixels)
559: More than 65% of reference points have a match:    Ok (71.88208770751953125%)
559: More than 65% of secondary points have a match:    Ok (68.1720428466796875%)
559: More than 95% of matches are good:                 Ok (99.684539794921875% at 0.5 pixel accuracy)
559:  -> Test EXIT SUCCESS.
559: -------------  No control baseline tests    -------------
1/1 Test #559: feTvKeyPointsAlgorithmsTest ......   Passed    7.97 sec

This allowed me to remove:

  • 10 test source code files
  • 16 tests
  • 19 baseline files from OTB-Data

Icing on the cake:

  • Testing of those feature no longer relies on baselines comparisons,
  • Number of test does no longer depends on OTB_USE_SIFTFAST.

Copyright

The copyright owner is CNES and has signed the ORFEO ToolBox Contributor License Agreement.


Check before merging:

  • All discussions are resolved
  • At least 2 :thumbsup: votes from core developers, no :thumbsdown: vote.
  • The feature branch is (reasonably) up-to-date with the base branch
  • Dashboard is green
  • Copyright owner has signed the ORFEO ToolBox Contributor License Agreement
Edited by Julien Michel

Merge request reports

Approval is optional

Merged by Julien MichelJulien Michel 6 years ago (Jan 21, 2019 12:00pm UTC)

Merge details

  • Changes merged into develop with d59f8a34.
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Julien Michel resolved all discussions

    resolved all discussions

  • I had a look at the test and it seems good to me.

  • This is great!

  • Luc Hermitte
  • Julien Michel resolved all discussions

    resolved all discussions

  • Julien Michel added 2 commits

    added 2 commits

    Compare with previous version

  • Can you add a test for KeyPointSetsMatchingFilter?
    As those tests are validation tests you need to make sure that two errors cannot make an acceptable result. Here an error in Sift or Surf combine with an error in the matching filter can lead to one of those results.

  • @regimbeau, maybe I can set absolute threshold on number of points / number of matches instead of threshold on ratio of matches / good matches ? That way we do not need more testing on matching filter (but if the keypoint filter fails we will not know the status of matching filter).

    I can also add a small test with hardcoded points to match.

  • Yes and that is why I think it would be good to have a test on the matching filter alone.
    And sure if you do so you can remove the matching filter in the keypoint filter tests! That it might be a bit quicker!

  • Ok I will add a small, no IOs test for the matching filter. I would like to keep matching in the keypoint filters test however: there is not much you can check on a single keypoint output, the fact that you can match them with a target performance validates that they are actually working correctly, and not outputing garbage points and descriptors.

  • Julien Michel resolved all discussions

    resolved all discussions

  • Julien Michel added 3 commits

    added 3 commits

    • 426b77cd - TEST: Add a small unit test without IO for matching filter (from MR review)
    • b5ba8001 - TEST: Reduce input size for better test duration
    • 6abaf1f3 - TEST: Add extra checks on number of points in reference and secondary image

    Compare with previous version

  • I did the following changes:

    • In the same test, add an IO-free test on matching filter, with hard-coded points
    • Reduce input image to 50x50 pixels to speed-up test
    • Add extra checks on number of points in reference and secondary images

    New test output is:

    559: 2019-01-14 16:15:31 (INFO): Default RAM limit for OTB is 128 MB
    559: 2019-01-14 16:15:31 (INFO): GDAL maximum cache size is 3208 MB
    559: 2019-01-14 16:15:31 (INFO): OTB will use at most 16 threads
    559: Checking matching filter:
    559: =========================
    559: Matches without backmatching: 
    559: [1, 1] <-> [5, 5]
    559: [2, 2] <-> [5, 5]
    559: [3, 3] <-> [6, 6]
    559: Matches with backmatching: 
    559: [2, 2] <-> [5, 5]
    559: [3, 3] <-> [6, 6]
    559: Without backmatching, the number of matches is 3:	Ok
    559: Without backmatching, p1 matches with p5:		Ok
    559: Without backmatching, p2 matches with p5:		Ok
    559: Without backmatching, p3 matches with p6:		Ok
    559: With backmatching, the number of matches is 2:	Ok
    559: With backmatching, p2 matches with p5:		Ok
    559: With backmatching, p3 matches with p6:		Ok
    559: 2019-01-14 16:15:31 (INFO): Loading kwl metadata from official product in file /work/scratch/michelj/dev/otb-data/Input/QB_TOULOUSE_RpcTag_100_100.tif
    559: Secondary image generated by applying a rotation of 2.5 degrees and scaling of 0.989999999999999991118215802999.
    559: Checking Surf implementation:
    559: =============================
    559: Found 98 points in reference image and 103 points in secondary image
    559: Found 14 matches with 9 valid matches (tolerance of 0.5 pixels)
    559: More than 95 points found in reference image:	Ok (98)
    559: More than 95 points found in secondary image:	Ok (103)
    559: More than 13% of reference points have a match:	Ok (14.2857151031494140625%)
    559: More than 13% of secondary points have a match:	Ok (13.59223270416259765625%)
    559: More than 64% of matches are good:             	Ok (64.28571319580078125% at 0.5 pixel accuracy)
    559: Checking Sift implementation:
    559: =============================
    559: Found 135 points in reference image and 128 points in secondary image
    559: Found 60 matches with 50 valid matches (tolerance of 0.5 pixels)
    559: More than 120 points found in reference image:	Ok (135)
    559: More than 120 points found in secondary image:	Ok (128)
    559: More than 44% of reference points have a match:	Ok (44.444446563720703125%)
    559: More than 44% of secondary points have a match:	Ok (46.875%)
    559: More than 82% of matches are good:             	Ok (83.3333282470703125% at 0.5 pixel accuracy)
    559: Checking SiftFast implementation:
    559: =================================
    559: Found 110 points in reference image and 117 points in secondary image
    559: Found 86 matches with 86 valid matches (tolerance of 0.5 pixels)
    559: More than 100 points found in reference image:	Ok (110)
    559: More than 100 points found in secondary image:	Ok (117)
    559: More than 73% of reference points have a match:	Ok (78.18182373046875%)
    559: More than 73% of secondary points have a match:	Ok (73.5042724609375%)
    559: More than 95% of matches are good:             	Ok (100% at 0.5 pixel accuracy)
    559:  -> Test EXIT SUCCESS.
    559: -------------  No control baseline tests    -------------
    1/1 Test #559: feTvKeyPointsAlgorithmsTest ......   Passed    1.26 sec
  • Side note: this would be easier to write with a unit test library ...

  • Side note: this would be easier to write with a unit test library ...

    I did introduce a dependency to boost.test, but do not hesitate to migrate to google.test or to catch2 if you feel like it :)

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading