[vtk-developers] DoublePi proposal
Bill Lorensen
bill.lorensen at gmail.com
Mon May 14 12:38:18 EDT 2012
David,
I'd like to revive this topic. I'm willing to take it over and track
down the regression failures.
A quick survey of (only .cxx) files:
vtkMath::Pi() - 64 files
vtkMath::TwoPi() - 0
vtkMath::DoublePi() - 13
vtkMath::DoubleTwoPi() = 4
3.1415... - 19
6.2631... - 5
Bill
On Wed, Apr 18, 2012 at 11:22 AM, David Gobbi <david.gobbi at gmail.com> wrote:
> An update on changing vtkMath::Pi() so that it returns "double"
> instead of "float":
>
> I pushed a patch to gerrit that implemented this change, but the
> extra precision causes a few tests to fail. Many of the sources and
> parametric functions in VTK use vtkMath::Pi(), and the few extra
> digits of precision cause them to produce slightly different results.
> Because not all tests are running with VTK 6 yet, I checked for test
> failures with a patched VTK 5.10. The following tests failed:
>
> CellLocator - ERROR: 202 ray-sphere intersections missed!!!
> progGlyphsBySource - central glyph is missing a piece
> SurfacePickerWithTexture - slight texture map change
> TexturedSphere - ditto (i.e. exactly the same diff)
> TestConeLayoutStrategy - small differences in text positions
>
> The last three failures are of the expected kind, i.e. slight changes
> in positions due to added precision. The CellLocator test failure
> is probably similar, but I need to investigate it further. The
> progGlyphsBySource error is the most confusing one... it might
> just be a depth buffer issue, or it could be a math error in the
> vtkSuperquadricSource.
>
> In any case, I think that I'll have to delay the merge until all the
> tests are running again.
>
> - David
>
> On Thu, Mar 1, 2012 at 10:14 AM, Marcus D. Hanwell
> <marcus.hanwell at kitware.com> wrote:
>> +1 on the DoublePi to Pi with double proposal, seems like a good place
>> to clean up.
>>
>> On Thu, Mar 1, 2012 at 11:46 AM, Philippe Pebay
>> <philippe.pebay at kitware.com> wrote:
>>> Agreed.
>>>
>>> Philippe
>>>
>>>
>>> On Thu, Mar 1, 2012 at 4:44 PM, Berk Geveci <berk.geveci at kitware.com> wrote:
>>>>
>>>> +1
>>>>
>>>> I would get rid of DoublePi in 6.0 since we are cleaning up so much stuff.
>>>>
>>>> On Thu, Mar 1, 2012 at 12:57 AM, David Gobbi <david.gobbi at gmail.com>
>>>> wrote:
>>>>>
>>>>> Well, I looked through VTK to see how vtkMath::Pi() was being used,
>>>>> and now I don't think that there is any reason for a single-precision
>>>>> pi at all. In VTK 6, vtkMath::Pi() should return a double-precision
>>>>> value, period.
>>>>>
>>>>> I found 62 classes that call vtkMath::Pi(). Only one of them,
>>>>> vtkExodusReader, seemed to have a reason for using a single-precision
>>>>> value. The others should have switched to vtkMath::DoublePi() a long
>>>>> time ago. My only guess as to why they weren't is that they were
>>>>> missed during the big float-to-double conversion that occurred when
>>>>> VTK 4.4 was released.
>>>>>
>>>>> So my new proposal is to do nothing for VTK 5.10 (because
>>>>> vtkMath::FloatPi() isn't really needed for anything), but to still
>>>>> change vtkMath::Pi() to "double" for VTK 6. Then eventually
>>>>> vtkMath::DoublePi() can be deprecated.
>>>>>
>>>>> - David
>>>>>
>>>>>
>>>>>
>>>>> On Wed, Feb 29, 2012 at 5:17 PM, David Gobbi <david.gobbi at gmail.com>
>>>>> wrote:
>>>>> > Hi J-C,
>>>>> >
>>>>> > All of those are good examples of why vtkMath:Pi() should be
>>>>> > returning double instead of float... in each of those four locations,
>>>>> > a double-precision Pi should be used instead of a single-precision Pi.
>>>>> > So don't changed them to FloatPi().
>>>>> >
>>>>> > I've also noticed that many of the vtkParametricFunction subclasses in
>>>>> > VTK are using vtkMath::Pi() when they should be using double-precision
>>>>> > pi.
>>>>> >
>>>>> > - David
>>>>> >
>>>>> >
>>>>> > On Wed, Feb 29, 2012 at 4:44 PM, Jean-Christophe Fillion-Robin
>>>>> > <jchris.fillionr at kitware.com> wrote:
>>>>> >> Within Slicer vtkMath::Pi() is called at four location. See details
>>>>> >> below.
>>>>> >>
>>>>> >> We will make sure to use vtkMath::FloatPi() when we will integrate VTK
>>>>> >> 5.10.
>>>>> >>
>>>>> >> Thanks
>>>>> >> Jc
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> Modules/Loadable/Annotations/VTKWidgets/vtkAnnotationGlyphSource2D.cxx
>>>>> >> 352: theta = 2.0 * vtkMath::Pi() / 8.0;
>>>>> >>
>>>>> >> Libs/MRML/Logic/vtkMRMLSliceLogic.cxx
>>>>> >> 2128: double
>>>>> >> axisMisalignmentDegrees=acos(dotProd)*180.0/vtkMath::Pi();
>>>>> >>
>>>>> >> Libs/vtkTeem/vtkPreciseHyperStreamline.cxx
>>>>> >> 591: vtkFloatingPointType
>>>>> >> theta=2.0*vtkMath::Pi()/this->NumberOfSides;
>>>>> >>
>>>>> >> Base/Logic/vtkSlicerGlyphSource2D.cxx
>>>>> >> 346: theta = 2.0 * vtkMath::Pi() / 8.0
>>>>> >>
>>>>> >>
>>>>> >> On Wed, Feb 29, 2012 at 6:41 PM, Jean-Christophe Fillion-Robin
>>>>> >> <jchris.fillionr at kitware.com> wrote:
>>>>> >>>
>>>>> >>> +1 :)
>>>>> >>>
>>>>> >>>
>>>>> >>> On Wed, Feb 29, 2012 at 4:57 PM, David Gobbi <david.gobbi at gmail.com>
>>>>> >>> wrote:
>>>>> >>>>
>>>>> >>>> Hi All,
>>>>> >>>>
>>>>> >>>> I'm wondering if, for VTK 6, we can make the vtkMath::Pi() return a
>>>>> >>>> double instead of a float. Currently, most VTK code is calling
>>>>> >>>> vtkMath::DoublePi() and, in my opinion, vtkMath::Pi() is more
>>>>> >>>> readable.
>>>>> >>>>
>>>>> >>>> Here's my proposal:
>>>>> >>>>
>>>>> >>>> 1) In VTK 5.10, a vtkMath::FloatPi() method would be added. Any VTK
>>>>> >>>> 5.10 code that calls vtkMath::Pi() would be changed to call
>>>>> >>>> vtkMath::FloatPi() instead.
>>>>> >>>>
>>>>> >>>> 2) In VTK 6, vtkMath::Pi() would be changed so that it returns
>>>>> >>>> double,
>>>>> >>>> and then all instances of vtkMath::DoublePi() can be changed to
>>>>> >>>> vtkMath::Pi() to improve readability.
>>>>> >>>>
>>>>> >>>> If anyone thinks this is a good idea, I can submit a patch for the
>>>>> >>>> VTK
>>>>> >>>> 5.10 changes to gerrit.
>>>>> >>>>
>>>>> >>>>
>>>>> >>>> To reiterate what I'm proposing, in VTK 5.10, the following methods
>>>>> >>>> would
>>>>> >>>> exist:
>>>>> >>>>
>>>>> >>>> double vtkMath::DoublePi();
>>>>> >>>> float vtkMath::Pi();
>>>>> >>>> float vtkMath::FloatPi(); - new method for single precision
>>>>> >>>>
>>>>> >>>> In VTK 6.0, the following methods would exist:
>>>>> >>>>
>>>>> >>>> double vtkMath::DoublePi();
>>>>> >>>> double vtkMath::Pi();
>>>>> >>>> float vtkMath::FloatPi();
>>>>> >>>>
>>>>> >>>> - David
>>>>> >>>> _______________________________________________
>>>>> >>>> Powered by www.kitware.com
>>>>> >>>>
>>>>> >>>> Visit other Kitware open-source projects at
>>>>> >>>> http://www.kitware.com/opensource/opensource.html
>>>>> >>>>
>>>>> >>>> Follow this link to subscribe/unsubscribe:
>>>>> >>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>>> >>>>
>>>>> >>>
>>>>> >>>
>>>>> >>>
>>>>> >>> --
>>>>> >>> +1 919 869 8849
>>>>> >>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> --
>>>>> >> +1 919 869 8849
>>>>> >>
>>>>> _______________________________________________
>>>>> Powered by www.kitware.com
>>>>>
>>>>> Visit other Kitware open-source projects at
>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>
>>>>> Follow this link to subscribe/unsubscribe:
>>>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>>
>>>> Visit other Kitware open-source projects at
>>>> http://www.kitware.com/opensource/opensource.html
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Philippe Pébay, PhD
>>> Director of Visualization and High Performance Computing /
>>> Directeur de la Visualisation et du Calcul Haute Performance
>>> Kitware SAS
>>> 26 rue Louis Guérin, 69100 Villeurbanne, France
>>> +33 (0) 6.83.61.55.70 / 4.37.45.04.15
>>> http://www.kitware.fr
>>>
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>
>>>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://www.vtk.org/mailman/listinfo/vtk-developers
>
--
Unpaid intern in BillsBasement at noware dot com
More information about the vtk-developers
mailing list