[PD-dev] getting double fixes into extra/

[katja]

On Tue, Nov 8, 2011 at 9:04 PM, Hans-Christoph Steiner <hans at at.or.at> wrote:
>
> Hey Katja,
>
> I was just reviewing the double precision patches for the extra/ section of pure-data.  I think we should try to get Miller to accept the 'extra/' fixes into pure-data.git now.  It seems to me that almost all of these changes are just float --> t_float, which are really no-brainers that would be really difficult to imagine causing problems.  Plus judging by all the tests you have written, it looks like your code is already pretty well tested.

In fact I only tested rewritten core functions intensively (on OSX and
Linux), for the extra's I just had a peek at the helpfiles so far, and
checked that there was no ridiculous output during normal use. So, I'm
not sure if it is wise to submit a patch file now. But I will try to
answer your questions below, one by one.


> There are only a couple changes in this patch that are not no-brainers.  The first is in sigmund~.c, it looks like you just moved up the #ifdef PD and #ifdef MSP blocks to the top of the file.  Seems harmless enough.

These blocks had to move up because t_float was otherwise not known by
the compiler, for the first part of the code. It is now no longer the
case that the first part can be used without modification in an
arbitrary application, like it used to be the case. An application
programmer using this code for anything else than Pd/MaxMsp, should
define t_float in another way. A comment could be added to mention
that t_float may be float or double?


Then there are a couple places where the code is using 'f' to force
single precision, like:
>
> @@ -677,8 +678,8 @@ static void bonk_doit(t_bonk *x)
>             {
>                 if (x->x_useloudness)
>                     growth += qrsqrt(qrsqrt(
> -                        power/(h->h_mask[oldmaskphase] + 1.0e-15))) - 1.f;
> -                else growth += power/(h->h_mask[oldmaskphase] + 1.0e-15) - 1.f;
> +                        power/(h->h_mask[oldmaskphase] + 1.0e-15))) - 1.;
> +                else growth += power/(h->h_mask[oldmaskphase] + 1.0e-15) - 1.;
>             }
>             if (!x->x_willattack && countup >= x->x_masktime)
>                 maskpow *= x->x_maskdecay;

I removed float suffixes consistently from code which I came across.
In the case of 1., 2. etc. this is of no consequence for the actual
value, but for other numbers it is. For example a float 0.001 cast to
double precision becomes 0.0010000000474974513, while the double could
be precise almost up to it's last digit. There's a nice online applet
'IEEE 754 Converter' on
http://www.h-schmidt.net/FloatApplet/IEEE754.html, showing such
rounding effects. One could use the float suffix to make sure no
redundant typecasts are done, but in the case of code compilable for
single and double precision, it has no advantage to force single
precision.

I have seen it stated on internet that something like 'float f = 1.0'
cannot be compiled in C++, it should be 'float f = 1.0f'. However, in
practice I've not met such problems with C++. Hope this is true on any
platform (otherwise we're stuck).


> And cases where there are added typedefs which I don't really understand what's going on, like:
>
> --- extra_original/expr~/vexp.h 2011-09-06 11:13:12.000000000 +0200
> +++ extra_double_ready/expr~/vexp.h     2011-09-06 11:13:12.000000000 +0200
> @@ -37,6 +37,7 @@
>  #else /* MSP */
>  #include "ext.h"
>  #include "z_dsp.h"
> +typedef float t_float;      // t_float is from m_pd.h
>  #endif
>
>  #include "fts_to_pd.h"
> --- extra_original/fiddle~/fiddle~.c    2010-04-26 00:27:35.000000000 +0200
> +++ extra_double_ready/fiddle~/fiddle~.c        2011-09-06 11:36:28.000000000 +0200
> @@ -108,11 +108,11 @@ static fts_symbol_t *dsp_symbol = 0;
>  #endif /* MSP */
>
>  #ifdef MSP
> -#define t_floatarg double
>  #include "ext.h"
>  #include "z_dsp.h"
>  #include "fft_mayer.proto.h"
> -
> +typedef float t_float;
> +typedef double t_floatarg;
>  #endif /* MSP */
>
>  #include <math.h>
> --- extra_original/loop~/loop~.c        2010-07-28 22:55:17.000000000 +0200
> +++ extra_double_ready/loop~/loop~.c    2011-09-06 11:33:54.000000000 +0200
> @@ -14,7 +14,8 @@ This file is downloadable from http://ww
>  #ifdef PD
>  #include "m_pd.h"
>  #else
> -#define t_sample float
> +typedef float t_float;
> +typedef float t_sample;
>  #endif
>
> --- extra_original/pd~/pd~.c    2010-07-28 22:55:17.000000000 +0200
> +++ extra_double_ready/pd~/pd~.c        2011-09-06 11:38:12.000000000 +0200
> @@ -26,7 +26,7 @@
>  #include "ext_support.h"
>  #include "ext_proto.h"
>  #include "ext_obex.h"
> -
> +typedef float t_float;
>  typedef double t_floatarg;
>  #define w_symbol w_sym
>  #define A_SYMBOL A_SYM


Ah these typedefs are nonsense indeed, t_float and t_floatarg are
defined in msp's zdsp.h as well, at least since Max/Msp5, earlier
versions I don't know.


Katja