VTK image error 0 despite differences - false pass

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

VTK image error 0 despite differences - false pass

Marcus D. Hanwell-2
Hi,

A while ago I opened issue https://gitlab.kitware.com/vtk/vtk/issues/17201 that talked about a bug on Windows where setting the precision to 0 resulted in 6 precision on Windows. There was a test, but it was passing even on Windows, this week we had a new employee start and I thought it would be a good first bug to create a failing test, and then make it pass by addressing the issue.

I thought that TestAxes was a little crowded, and that had perhaps been enough to throw the image diff off. So he wrote a test that just had this axis in, made a baseline for what was expected, and then changed it so that we could see the test fail. Changing to 6 or 8 is still showing passing test:



This looks like a good difference to me (not sure if we are allowed attachments, but I tried). The ImageError is being shown as 0. We could make the text bigger, but there are a few chart tests where we use text of this size and I thought they would fail with extra numbers like this.

Thanks,

Marcus

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: VTK image error 0 despite differences - false pass

Marcus D. Hanwell-2
On Fri, May 4, 2018 at 9:34 AM, Marcus D. Hanwell <[hidden email]> wrote:
Hi,

A while ago I opened issue https://gitlab.kitware.com/vtk/vtk/issues/17201 that talked about a bug on Windows where setting the precision to 0 resulted in 6 precision on Windows. There was a test, but it was passing even on Windows, this week we had a new employee start and I thought it would be a good first bug to create a failing test, and then make it pass by addressing the issue.

I thought that TestAxes was a little crowded, and that had perhaps been enough to throw the image diff off. So he wrote a test that just had this axis in, made a baseline for what was expected, and then changed it so that we could see the test fail. Changing to 6 or 8 is still showing passing test:



This looks like a good difference to me (not sure if we are allowed attachments, but I tried). The ImageError is being shown as 0. We could make the text bigger, but there are a few chart tests where we use text of this size and I thought they would fail with extra numbers like this.

Digging into this for anyone following, I can change vtkTesting.cxx to use 'rtId->SetAllowShift(0);' which removes the single pixel shift logic, and I get a diff showing my extra digits, as I would expect. We likely have this on for a reason, and the text is pretty tiny, so I could see a nearest neighbor search turning up no difference for every character added.

As the docs say this is not symmetric - the testing does shoe a diff if the image on the right is the baseline, and the characters get removed.

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: VTK image error 0 despite differences - false pass

Marcus D. Hanwell-2
On Fri, May 4, 2018 at 1:24 PM, Marcus D. Hanwell <[hidden email]> wrote:
On Fri, May 4, 2018 at 9:34 AM, Marcus D. Hanwell <[hidden email]> wrote:
Hi,

A while ago I opened issue https://gitlab.kitware.com/vtk/vtk/issues/17201 that talked about a bug on Windows where setting the precision to 0 resulted in 6 precision on Windows. There was a test, but it was passing even on Windows, this week we had a new employee start and I thought it would be a good first bug to create a failing test, and then make it pass by addressing the issue.

I thought that TestAxes was a little crowded, and that had perhaps been enough to throw the image diff off. So he wrote a test that just had this axis in, made a baseline for what was expected, and then changed it so that we could see the test fail. Changing to 6 or 8 is still showing passing test:



This looks like a good difference to me (not sure if we are allowed attachments, but I tried). The ImageError is being shown as 0. We could make the text bigger, but there are a few chart tests where we use text of this size and I thought they would fail with extra numbers like this.

Digging into this for anyone following, I can change vtkTesting.cxx to use 'rtId->SetAllowShift(0);' which removes the single pixel shift logic, and I get a diff showing my extra digits, as I would expect. We likely have this on for a reason, and the text is pretty tiny, so I could see a nearest neighbor search turning up no difference for every character added.

As the docs say this is not symmetric - the testing does shoe a diff if the image on the right is the baseline, and the characters get removed.

Lots of tests fail when this is turned off, we can actually make the text larger and see the expected failure in the axis test. We could redo the TestAxes to use a larger font, or perhaps add an option to disable the pixel shift on a per test basis (I didn't see anything exposing that in the test driver.

It is something people should be aware of, and it is unfortunate our default text size appears to be small enough that obviously different images will show a zero error. Thoughts? Anything we missed when looking into this?

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: VTK image error 0 despite differences - false pass

Marcus D. Hanwell-2
On Fri, May 4, 2018 at 1:59 PM, Marcus D. Hanwell <[hidden email]> wrote:
On Fri, May 4, 2018 at 1:24 PM, Marcus D. Hanwell <[hidden email]> wrote:
On Fri, May 4, 2018 at 9:34 AM, Marcus D. Hanwell <[hidden email]> wrote:
A while ago I opened issue https://gitlab.kitware.com/vtk/vtk/issues/17201 that talked about a bug on Windows where setting the precision to 0 resulted in 6 precision on Windows. There was a test, but it was passing even on Windows, this week we had a new employee start and I thought it would be a good first bug to create a failing test, and then make it pass by addressing the issue.

I thought that TestAxes was a little crowded, and that had perhaps been enough to throw the image diff off. So he wrote a test that just had this axis in, made a baseline for what was expected, and then changed it so that we could see the test fail. Changing to 6 or 8 is still showing passing test:



This looks like a good difference to me (not sure if we are allowed attachments, but I tried). The ImageError is being shown as 0. We could make the text bigger, but there are a few chart tests where we use text of this size and I thought they would fail with extra numbers like this.

Digging into this for anyone following, I can change vtkTesting.cxx to use 'rtId->SetAllowShift(0);' which removes the single pixel shift logic, and I get a diff showing my extra digits, as I would expect. We likely have this on for a reason, and the text is pretty tiny, so I could see a nearest neighbor search turning up no difference for every character added.

Lots of tests fail when this is turned off, we can actually make the text larger and see the expected failure in the axis test. We could redo the TestAxes to use a larger font, or perhaps add an option to disable the pixel shift on a per test basis (I didn't see anything exposing that in the test driver.

For anyone interested, a merge request with dashboard results looking at the impact of not allowing shifts in image diffs globally,


_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://vtk.org/mailman/listinfo/vtk-developers

Reply | Threaded
Open this post in threaded view
|

Re: VTK image error 0 despite differences - false pass

Marcus D. Hanwell-2
On Fri, May 4, 2018 at 2:08 PM, Marcus D. Hanwell <[hidden email]> wrote:
On Fri, May 4, 2018 at 1:59 PM, Marcus D. Hanwell <[hidden email]> wrote:
On Fri, May 4, 2018 at 1:24 PM, Marcus D. Hanwell <[hidden email]> wrote:
On Fri, May 4, 2018 at 9:34 AM, Marcus D. Hanwell <[hidden email]> wrote:
A while ago I opened issue https://gitlab.kitware.com/vtk/vtk/issues/17201 that talked about a bug on Windows where setting the precision to 0 resulted in 6 precision on Windows. There was a test, but it was passing even on Windows, this week we had a new employee start and I thought it would be a good first bug to create a failing test, and then make it pass by addressing the issue.

I thought that TestAxes was a little crowded, and that had perhaps been enough to throw the image diff off. So he wrote a test that just had this axis in, made a baseline for what was expected, and then changed it so that we could see the test fail. Changing to 6 or 8 is still showing passing test:



This looks like a good difference to me (not sure if we are allowed attachments, but I tried). The ImageError is being shown as 0. We could make the text bigger, but there are a few chart tests where we use text of this size and I thought they would fail with extra numbers like this.

Digging into this for anyone following, I can change vtkTesting.cxx to use 'rtId->SetAllowShift(0);' which removes the single pixel shift logic, and I get a diff showing my extra digits, as I would expect. We likely have this on for a reason, and the text is pretty tiny, so I could see a nearest neighbor search turning up no difference for every character added.

Lots of tests fail when this is turned off, we can actually make the text larger and see the expected failure in the axis test. We could redo the TestAxes to use a larger font, or perhaps add an option to disable the pixel shift on a per test basis (I didn't see anything exposing that in the test driver.

For anyone interested, a merge request with dashboard results looking at the impact of not allowing shifts in image diffs globally,


For anyone following along, I wrote up  a blog post summarizing the issue, linking to this thread, and the resolution,


It turned up a number of issues that had been lurking with passing tests that should have been failing. You may want to be aware if you reuse this mechanism too.

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
https://public.kitware.com/mailman/listinfo/vtk-developers