search

Ashwinning / libmv

Automatically exported from code.google.com/p/libmv
MIT License
0 stars 0 forks source link

Several tests failing after following README instructions #29

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Check out latest version.
2. make
3. make release
4. cd bin-dbg
5. make test

What is the expected output? What do you see instead?

All test should pass. Several of the tests are failing:

The following tests FAILED:
     19 - image_converter_test (Failed)
     20 - image_transform_linear_test (OTHER_FAULT)
     46 - homography_test (Failed)
     50 - panography_test (Failed)
     75 - similarity_parameterization_test (Failed)
     79 - euclidean_parameterization_test (Failed)
Errors while running CTest
make: *** [test] Error 8

What version of the product are you using? On what operating system?

I downloaded the tarball libmv-libmv-7bef9ca.tar.gz and am using Ubuntu 10.10.

Please provide any additional information below.

I am keen to help resolve this issue but will need some guidance from the 
developers as to where to look for the problem, or what the problem may be..

Original issue reported on code.google.com by stephen....@gmail.com on 2 Aug 2011 at 2:06

GoogleCodeExporter commented 9 years ago 
Hi, yes it remains some known bugs to solve, and any help is welcomed!
So, some details I know about these bugs:
* 19 - image_converter_test (Failed)
This is due to an approximation of char image: expected = 127, actual = 126...
* 20 - image_transform_linear_test (OTHER_FAULT)
This is mainly due a wrong estimation of the ground truth (bounding box, etc.), 
which is not the good one I think.
* 46 - homography_test (Failed)
There is a bug when we estimate the 2D homography with 4 points.
* 50 - panography_test (Failed)
Need investigations.
* 75 - similarity_parameterization_test (Failed)
I don't have this issue, can you send us the description?
* 79 - euclidean_parameterization_test (Failed)
I don't have this issue, can you send us the description?

Original comment by julien.michot.fr@gmail.com on 7 Aug 2011 at 10:13

GoogleCodeExporter commented 9 years ago 
Hi Julien

Sure - but what I provided you is all that appears on the console. How can I 
get a more detailed description? Is there a verbose mode for testing, or a log 
file that gets generated somewhere? I'm not too familiar with how the testing 
framework operates.. I tried adding some debug code in 
"euclidean_parameterization_test.cc" but when I recompile and attempt "make 
test" again, I can't see any of my debug lines appear..

Original comment by stephen....@gmail.com on 7 Aug 2011 at 11:26

GoogleCodeExporter commented 9 years ago 
Hi,
You can run separatly the tests, for instance: 
./bin/tests/euclidean_parameterization_test -v 2 --alsologtostderr

"--alsologtostderr" provides the debug info in the shell.

Original comment by julien.michot.fr@gmail.com on 10 Aug 2011 at 11:17

GoogleCodeExporter commented 9 years ago 
Thanks Julien. OK, when I run #75 and #79 I get the following output:

/-----------------------------------/

RUN (75)
    ./similarity_parameterization_test -v 2 --alsologtostderr

ERR (75)
    src/libmv/multiview/similarity_parameterization_test.cc:63: Failure
    The difference between 0 and s is 2.1073424255447017e-08, which exceeds 1.5e-8, where
    0 evaluates to 0, s evaluates to 2.1073424255447017e-08, and 1.5e-8 evaluates to 1.4999999999999999e-08.

    [  FAILED  ] Similarity2DSAParameterization.Roundtripping (1 ms)

    [  FAILED  ] 1 test, listed below:
    [  FAILED  ] Similarity2DSAParameterization.Roundtripping

    1 FAILED TEST

RUN (79)
    ./euclidean_parameterization_test -v 2 --alsologtostderr

ERR (79)
    src/libmv/multiview/euclidean_parameterization_test.cc:87: Failure
    The difference between 0 and s is 2.1073424255447017e-08, which exceeds 1.5e-8, where
    0 evaluates to 0, s evaluates to 2.1073424255447017e-08, and 1.5e-8 evaluates to 1.4999999999999999e-08.

    [  FAILED  ] Euclidean2DSCParameterization.Roundtripping (1 ms)

    [  FAILED  ] 1 test, listed below:
    [  FAILED  ] Euclidean2DSCParameterization.Roundtripping

    1 FAILED TEST

/-----------------------------------/

So I'm guessing the problem is that the matrix calculations (maybe on my 32-bit 
machine) are just not quite as accurate as the tests require...? I suppressed 
the errors on my install by relaxing the accuracy requirements here:

similarity_parameterization_test.cc:63
changed:
EXPECT_MATRIX_PROP(h, h_roundtrip, 1.5e-8);
to:
EXPECT_MATRIX_PROP(h, h_roundtrip, 2.5e-8);

euclidean_parameterization_test.cc:87
changed:
EXPECT_MATRIX_PROP(h, h_roundtrip, 1.5e-8);
to:
EXPECT_MATRIX_PROP(h, h_roundtrip, 2.5e-8);

Please let me know if there's anything else I can do to help with these issues.

Original comment by stephen....@gmail.com on 12 Aug 2011 at 3:57

GoogleCodeExporter commented 9 years ago 
Hi, yes let's decrease a little the expected precision.
For the next issues, you will need to investigate but the 19 is maybe the 
easiest one?

Original comment by julien.michot.fr@gmail.com on 14 Aug 2011 at 10:16

GoogleCodeExporter commented 9 years ago 
Hey

OK I've had a look at 19. It only seems to be failing when using the Array3Du 
(Uchar) datatype, and I think I know the problem. It basically seems to be a 
casting issue where the result gets rounded down when it's entered directly 
into the static cast line:

----ORIGINAL FUNCTION (from src/libmv/image/image_converter.h)
template<typename T>
inline T RGB2GRAY(const T r,const T g, const T b) {
  return static_cast<T>(r * 0.2126 + g * 0.7152 + b * 0.0722);
}
----END ORIGINAL FUNCTION

I managed to avoid the error by assigning the result of the equation to a 
"double" variable and then using that variable:

----IMPROVED FUNCTION:
template<typename T>
inline T RGB2GRAY(const T r,const T g, const T b) {
    double val = r * 0.2126 + g * 0.7152 + b * 0.0722;
    return static_cast<T>(val);
}
----END IMPROVED FUNCTION

Is that a satisfactory fix? It actually works with a float type as well, but I 
thought the couple of bytes of memory saved was not worth it..

When I have time I will explore the other issues.

Original comment by stephen....@gmail.com on 15 Aug 2011 at 6:22

GoogleCodeExporter commented 9 years ago 
Haha actually I couldn't restrain myself from starting with the next one. Here 
is my progress so far:

// ----- ERROR 20 PART 1 ----- //
/src/libmv/image/image_transform_linear_test.cc:82: Failure
Value of: 1.0
  Actual: 1
Expected: image_rot(i,y)
Which is: 0
[  FAILED  ] ImageTransform.RotateImage90 (1 ms)
// --------------------------- //

// ----- ERROR 20 PART 2 ----- //
image_transform_linear_test: /src/libmv/image/image_transform_linear.cc:73: 
void libmv::ResizeImage(const libmv::Vec2u&, const libmv::Mat3&, 
libmv::FloatImage*, libmv::Mat3*, libmv::Vec4i*): Assertion `(*bbox_ptr)(1) < 
(*bbox_ptr)(0)' failed.
Aborted
// --------------------------- //

// ----- PART 1 DIAGNOSIS ----- //
src/libmv/image/image_transform_linear_test.cc:

The section

  for (int i = 0; i < h; ++i){
    EXPECT_EQ(image_rot(i,y), 1.0);
  }

needs to be changed to:

  for (int i = 0; i < h; ++i){
    EXPECT_EQ(image_rot(i,h-1-y), 1.0);
  }

The tests for the anti-clockwise (positive) and clockwise (negative) rotations 
need to check different columns for ones because the two results will only be 
identical in the special case of an ODD x ODD matrix with the line exactly half 
way down. However, this introduces some other errors, which I think are because 
the rotation function in <image_transform_linear.cc> is incorrect in that it 
does not assign the centre of rotation as the dead centre of the matrix. To 
correct this, the following needs to be changed in RotateImage():

  Ht << 1, 0, -image_in.Height()/2.0,
        0, 1, -image_in.Width()/2.0,
        0, 0,  1;

To

  Ht << 1, 0, -image_in.Height()/2.0,
        0, 1, -image_in.Width()/2.0,
        0, 0,  1;

This invalidates some of the tests, which then need to be changed - such as 
TEST(ImageTransform, RotateImage45). I think this needs an odd-sized matrix to 
simplify the test (so that the centered horizontal line gets positioned 
directly on the diagonal):

CHANGE:

const int w = 10, h = 10;

TO:

const int w = 11, h = 11;

Having done this, all the problems associated with part 1 of the test failure 
seem to be gone! I tried my new code out with all the other tests and it 
doesn't seem to affect them...
// ---------------------------- //

For the second part of the test failure, I agree, it looks like the error is 
probably in void ComputeBoundingBox()... I should have some time over the next 
few days to address this.

Original comment by stephen....@gmail.com on 15 Aug 2011 at 7:59

GoogleCodeExporter commented 9 years ago 
Good!
The 19: I don't particularly like this way, but it's a quick and working fix, 
so ok.
Is the following way identical?
  return static_cast<T>(static_cast<double>(val));

The 20 p1: I don't see any difference in RotateImage():
 Ht <<  1, 0, -image_in.Height()/2.0,
        0, 1, -image_in.Width()/2.0,
        0, 0,  1;
To
  Ht << 1, 0, -image_in.Height()/2.0,
        0, 1, -image_in.Width()/2.0,
        0, 0,  1;

The 20 p2: cool, I will wait!
Can you make some patch so that i can fix the code?

Original comment by julien.michot.fr@gmail.com on 16 Aug 2011 at 8:03

GoogleCodeExporter commented 9 years ago 
Hi Julien

Yes for #19 your alternative works fine as well.

RE #20: Haha sorry I accidentally copy and pasted the same block - the second 
block should be:

  Ht << 1, 0, -(image_in.Height()-1)/2.0,
        0, 1, -(image_in.Width()-1)/2.0,
        0, 0,  1;

Would you agree? This is the way to shift to the dead centre of the image in 
most reference systems as far as I'm aware.

Do you want the patch to be for all of the changes I've suggested? Or just for 
#20? I'm a beginner with these sorts of collaborative projects, so I've never 
written a patch before, but I'm sure I can figure it out.

In any case, I don't fully understand what the test is trying to do in #20. 
TEST(ImageTransform, RescaleImageTranslation) seems to create a homography 
which only contains a translation, but then it expects the dimensions to change 
as if it were a scaling? Also, in the ResizeImage() function, the assertion 
applying to the bounding box dimensions seems to be wrong. Instead of:

  assert((*bbox_ptr)(1) < (*bbox_ptr)(0));
  assert((*bbox_ptr)(3) < (*bbox_ptr)(2));

I would have thought it should be:

  assert((*bbox_ptr)(1) > (*bbox_ptr)(0));
  assert((*bbox_ptr)(3) > (*bbox_ptr)(2));

Original comment by stephen....@gmail.com on 16 Aug 2011 at 10:37

GoogleCodeExporter commented 9 years ago 
Hi !

Well, as soon as these issues are solved, it can be a simple patch.
it can be done by: "git diff > libmv-issues-19.patch"

Cool for the last one. This bug is my fault, I haven't totally finished the 
tests :)

Original comment by julien.michot.fr@gmail.com on 18 Aug 2011 at 1:35

GoogleCodeExporter commented 9 years ago 
Hi Stephen,
any news about the patch?

Original comment by julien.michot.fr@gmail.com on 21 Sep 2011 at 7:24

GoogleCodeExporter commented 9 years ago 
Hi Julien

Sorry, I think at the time I thought you would only want the patch once all of 
the issues are solved. Since then I have been distracted and forgot about it.

Please find attached a patch that should solve #19, #75, #79 and part of #20.

The remainder of the problems are out of my ability, so I would need some hints 
if I were to tackle them myself.

Original comment by stephen....@gmail.com on 22 Sep 2011 at 3:47

Attachments:

GoogleCodeExporter commented 9 years ago 
Thanks for the patch!
I have also fixed the #20 but your bugfix for the #19 doesn't work on my linux 
32b (it works ok on windows).
Actually, it does work when I display the number (std::cout << d;) but not when 
I don't display it! 
o_O Strange..

The following tests don't pass
- #46, homography test
- #50, panography test
- #41, projection test (new one!)
- vector and graph tests on windows (new one!)

I will try to spend some time on them.
Thx for your work!

Original comment by julien.michot.fr@gmail.com on 22 Sep 2011 at 7:18