[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
changeset.patch
Description: Binary data
- Re: Qhull test changes, (continued)
- Re: Qhull test changes, Rik, 2012/01/30
- Re: Qhull test changes, Ben Abbott, 2012/01/30
- Re: Qhull test changes, Rik, 2012/01/30
- Re: Qhull test changes, Alexander Hansen, 2012/01/30
- Re: Qhull test changes, Ben Abbott, 2012/01/30
- Re: Qhull test changes, Brad Barber, 2012/01/30
- Re: Qhull test changes, Ben Abbott, 2012/01/30
- Re: Qhull test changes, Brad Barber, 2012/01/30
- Re: Qhull test changes, Ben Abbott, 2012/01/31
- Re: Qhull test changes, Brad Barber, 2012/01/31
- Re: Qhull test changes,
Ben Abbott <=
- Re: Qhull test changes, Brad Barber, 2012/01/30
- Re: Qhull test changes, Tatsuro MATSUOKA, 2012/01/30
- Re: Qhull test changes, Tatsuro MATSUOKA, 2012/01/31
- Re: Qhull test changes, Tatsuro MATSUOKA, 2012/01/31
- Re: Qhull include files, Rik, 2012/01/31
- Re: Qhull include files, Brad Barber, 2012/01/31
- Re: Qhull include files, Rik, 2012/01/31