[vtk-developers] DoublePi proposal

Wed Apr 18 11:22:45 EDT 2012

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
>>
>>