[PD-dev] getting double fixes into extra/

[Hans-Christoph Steiner]

On Nov 8, 2011, at 8:32 PM, katja wrote:

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


Ah I see, they are for Max/MSP only?  I didn't notice that before.  So I guess the thing to do would be for you do make adjustments in the pd-double.git, then we can rebase into a patch for just extra and submit it for Miller.  I think these changes are worth getting into pure-data.git now since in 0.42 Miller already went thru the core source code to make sure all of the t_float and t_sample types were used where they made sense. 

.hc


----------------------------------------------------------------------------

"We have nothing to fear from love and commitment." - New York Senator Diane Savino, trying to convince the NY Senate to pass a gay marriage bill