Failing cli-colorspace(12) test - double value equality

Questions and postings pertaining to the development of ImageMagick, feature enhancements, and ImageMagick internals. ImageMagick source code and algorithms are discussed here. Usage questions which are too arcane for the normal user list should also be posted here.
Post Reply
neuron
Posts: 12
Joined: 2016-06-24T05:03:47-07:00
Authentication code: 1151

Failing cli-colorspace(12) test - double value equality

Post by neuron » 2016-06-24T05:27:18-07:00

Hi,

I am trying to compile Imagemagick 6.9.4-9 on solaris 10. There are 7 tests failing

FAIL: tests/cli-colorspace.tap 12
FAIL: tests/validate-formats-disk.tap 1
FAIL: tests/validate-formats-map.tap 1
FAIL: tests/validate-formats-memory.tap 1
FAIL: tests/wandtest.tap 1
FAIL: Magick++/demo/demos.tap 3
FAIL: Magick++/demo/demos.tap 6

I started to look at the first failure, ignoring the rest for now. The test 12 is about HSL <-> sRGB conversion.
If I run the test by hand, I get "137,80,146" which is not equal to the original value "146,89,80".

It turns out that the problem is in comparing double values in ConvertRGBToHSL

The function says:

Code: Select all

...
  if (c <= 0.0)
    {
      *hue=0.0;
      *saturation=0.0;
      return;
    }
  if (max == (QuantumScale*red))
    {
      *hue=(QuantumScale*green-QuantumScale*blue)/c;
      if ((QuantumScale*green) < (QuantumScale*blue))
        *hue+=6.0;
    }
  else
...
The condition

Code: Select all

max == (QuantumScale*red)
does not match, because the difference between the values is -3.171291e-17.

I have replaced all the eqality comparisons with

Code: Select all

fabs(a-b) < 0.00001
(I would attach patch if I could find how) and the test is now passing. But I'm not sure whether it's generally good approach. Since you do a lot of floating math, I guess you had to tackle such problems in the past. What would you recommend to do to fix the issue?

Thank you
__
Vlad

neuron
Posts: 12
Joined: 2016-06-24T05:03:47-07:00
Authentication code: 1151

Re: Failing cli-colorspace(12) test - double value equality

Post by neuron » 2016-06-24T05:30:24-07:00

Ok, I quickly pasted the patch at http://pastebin.com/PHLAj7SF. It's just to show the approach I have taken.

User avatar
magick
Site Admin
Posts: 11043
Joined: 2003-05-31T11:32:55-07:00

Re: Failing cli-colorspace(12) test - double value equality

Post by magick » 2016-06-24T06:53:40-07:00

Thanks for the problem report. We can reproduce it and will have a patch to fix it in GIT master branch @ https://github.com/ImageMagick/ImageMagick later today. The patch will be available in the beta releases of ImageMagick @ http://www.imagemagick.org/download/beta/ by sometime tomorrow.

neuron
Posts: 12
Joined: 2016-06-24T05:03:47-07:00
Authentication code: 1151

Re: Failing cli-colorspace(12) test - double value equality

Post by neuron » 2016-06-24T06:59:38-07:00

Ok, thanks. I'm curious about what the fix will be, as floating point math is evil :)

User avatar
magick
Site Admin
Posts: 11043
Joined: 2003-05-31T11:32:55-07:00

Re: Failing cli-colorspace(12) test - double value equality

Post by magick » 2016-06-24T09:59:39-07:00

The fix is as you suggested. Its never been a problem in the past but best practices for floating point is to compare the results to an epsilon value. The patch should be in Github now.

neuron
Posts: 12
Joined: 2016-06-24T05:03:47-07:00
Authentication code: 1151

Re: Failing cli-colorspace(12) test - double value equality

Post by neuron » 2016-07-01T06:35:05-07:00

Thank you, works like a charm.

Post Reply