octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Qhull test changes


From: Ben Abbott
Subject: Re: Qhull test changes
Date: Mon, 30 Jan 2012 19:42:01 -0500

On Jan 30, 2012, at 6:02 PM, Rik wrote:

> On 01/30/2012 02:54 PM, Ben Abbott wrote:
> 
>> On Jan 30, 2012, at 3:09 PM, Rik wrote:
>> 
>>> On 01/30/2012 06:39 AM, Ben Abbott wrote:
>>>>> Rik,
>>>>> 
>>>>> I pushed a changeset to fix the tests.
>>>>> 
>>>>>      http://hg.savannah.gnu.org/hgweb/octave/rev/2633baa831e2
>>>>> 
>>>>> Ben
>>>>> 
>>> I now get an error the other direction.  See below.
>>> 
>>> ***** testif HAVE_QHULL
>>> cube = [0 0 0;1 0 0;1 1 0;0 1 0;0 0 1;1 0 1;1 1 1;0 1 1];
>>> [h, v] = convhulln (cube, "Qt");
>>> assert (size (h), [12 3]);
>>> h = sortrows (sort (h, 2), [1:3]);
>>> assert (h, [1 2 4; 1 2 6; 1 4 8; 1 5 6; 1 5 8; 2 3 4; 2 3 7; 2 6 7; 3 4 7;
>>> 4 7 8; 5 6 7; 5 7 8]);
>>> assert (v, 1, 10*eps);
>>> [h2, v2] = convhulln (cube); % Test defaut option = "Qt"
>>> assert (size (h2), size (h))
>>> h2 = sortrows (sort (h2, 2), [1:3]);
>>> assert (h2, h);
>>> assert (v2, v, 10*eps);
>>> !!!!! test failed
>>> assert (size (h),[12, 3]) expected
>>>  12    3
>>> but got
>>>  6   4
>>> values do not match
>>> 
>>> Previously the tests were matched to Qhull <= 2010.  Now the tests are
>>> matched to Qhull >= 2011.  In either case, if Octave is linked against the
>>> other version then a test error results and users are going to think it is
>>> Octave's fault.  This is absolutely up-to-date Octave code from Mercurial
>>> with your changeset, but linked against the default Qhull in Kubuntu 10.04
>>> which is version 2009.
>>> 
>>> A couple of quick ways to solve this.
>>> 
>>> 1) Don't check the result.  Check only that h = convhulln (..., "Qt")
>>> matches h2 = convhulln (...) which shows that Octave is passing Qt as the
>>> default option.
>>> 
>>> 2) Check the size of the result (12 x 3 or 6 x 4) and then compare against
>>> the appropriate array.
>>> 
>>> 3) Embody the test in configure.ac so that Octave can identify which
>>> version of Qhull it is linking against.  Then you could use two testif
>>> macros, say HAVE_QHULL or HAVE_QHULL_NEW.
>>> 
>>> Cheers,
>>> Rik
>> I'm not certain, but doesn't "Qt" imply that the convex hull should be made 
>> up of triangles ? (perhaps I should study the qhull docs a bit ?)
>> 
>> In any event, I favored the more recent qhull because it matches Matlab's 
>> result.
>> 
> Yes, the output should be triangulated when we pass the 'Qt' option and the
> new post-2011 Qhull behavior is mathematically correct.  The problem is
> that Qhull is not returning triangulated output for versions less than 2011
> and users will blame Octave when they see a failing test in the test
> report.  I am proposing that Octave work around the different Qhull
> versions so we don't generate a lot of spurious bug reports.
> 
> On the other hand, if we want we could leave the test in and also put in
> some comments that specifically say, "If you see this test failing, then
> you must upgrade your Qhull installation."  This might do a bit towards
> pushing users and distributions to upgrade to a new Qhull.
> 
> --Rik

Would it be sufficient to allow either result to pass?

How about the attached changeset ?

Ben

Attachment: changeset.patch
Description: Binary data






reply via email to

[Prev in Thread] Current Thread [Next in Thread]