Party Monster audio test

[Stefan Westerfeld]

   Hi!

Beast has an automated test framework (in the CVS version in
slowtests/audio), which allows us to test that the output of .bse files
stays the same after doing changes to the code. The process works by
extracting features (such as the spectrum) of the original test run and
the new test run, and comparing these.

When I tried to add partymonster.bse to the automated tests, however, I
saw that the output bsesh creates differs from run to run. Since
bsefcompare compares them with an algorithm similar to human perception,
they are still very close (far above 99%), and as human listener there
is no difference. But they are not completely the same, and do not meet
the 99.99% comparision threshold we normally use.

I investigated further, and the problem are modules that use random
data, so I did some hacks to eliminate the random data, to prove that
then the test passes like it should. So here is the patch, with which
the test passes:

Index: plugins/bsenoise.cc
===================================================================
RCS file: /cvs/gnome/beast/plugins/bsenoise.cc,v
retrieving revision 1.2
diff -u -p -r1.2 bsenoise.cc
--- plugins/bsenoise.cc 7 Mar 2005 07:40:39 -0000 1.2
+++ plugins/bsenoise.cc 24 May 2006 21:51:44 -0000
@@ -53,7 +53,8 @@ class Noise : public NoiseBase {
     {
       g_return_if_fail (n_values <= block_size()); /* paranoid */
 
-      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() % (noise_data->size() - n_values)]);
+      //ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() % (noise_data->size() - n_values)]);
+      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[0]);
     }
   };
 public:
@@ -66,8 +67,12 @@ public:
     const int N_NOISE_BLOCKS = 20;
     noise_data.resize (block_size() * N_NOISE_BLOCKS);
 
+    GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
+
     for (vector<gfloat>::iterator ni = noise_data.begin(); ni != noise_data.end(); ni++)
-      *ni = 1.0 - rand() / (0.5 * RAND_MAX);  // FIXME: should have class noise
+      *ni = g_rand_double_range (random_generator, -1, 1);
+
+    g_rand_free (random_generator);
   }
   void
   reset1()
Index: plugins/davsyndrum.c
===================================================================
RCS file: /cvs/gnome/beast/plugins/davsyndrum.c,v
retrieving revision 1.29
diff -u -p -r1.29 davsyndrum.c
--- plugins/davsyndrum.c 16 May 2006 10:20:47 -0000 1.29
+++ plugins/davsyndrum.c 24 May 2006 21:51:44 -0000
@@ -259,7 +259,7 @@ dmod_process (BseModule *module,
           /* trigger drum */
           dmod_trigger (dmod,
                         freq_in ? BSE_FREQ_FROM_VALUE (freq_in[i]) : dmod->params.freq,
-                        ratio_in ? ratio_in[1] : 1.0);
+                        ratio_in ? ratio_in[i] : 1.0); /* FIXME: <- bug? */
           spring_vel = dmod->spring_vel;
           env = dmod->env;
           freq_rad = dmod->freq_rad;
Index: plugins/davxtalstrings.c
===================================================================
RCS file: /cvs/gnome/beast/plugins/davxtalstrings.c,v
retrieving revision 1.33
diff -u -p -r1.33 davxtalstrings.c
--- plugins/davxtalstrings.c 8 May 2006 01:37:22 -0000 1.33
+++ plugins/davxtalstrings.c 24 May 2006 21:51:44 -0000
@@ -303,11 +303,17 @@ xmod_trigger (XtalStringsModule *xmod,
   /* Add some snap. */
   for (i = 0; i < xmod->size; i++)
     xmod->string[i] = pow (xmod->string[i], xmod->tparams.snap_factor * 10.0 + 1.0);
+
   
+  GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
+
   /* Add static to displacements. */
   for (i = 0; i < xmod->size; i++)
     xmod->string[i] = (xmod->string[i] * (1.0F - xmod->tparams.metallic_factor) +
-       (bse_rand_bool () ? -1.0F : 1.0F) * xmod->tparams.metallic_factor);
+       (g_rand_boolean(random_generator) ? -1.0F : 1.0F) * xmod->tparams.metallic_factor);
+  
+  g_rand_free (random_generator);
+
   /* Set velocity. */
   for (i = 0; i < xmod->size; i++)
     xmod->string[i] *= xmod->tparams.trigger_vel;
Index: slowtests/audio/Makefile.am
===================================================================
RCS file: /cvs/gnome/beast/slowtests/audio/Makefile.am,v
retrieving revision 1.20
diff -u -p -r1.20 Makefile.am
--- slowtests/audio/Makefile.am 21 May 2006 18:04:12 -0000 1.20
+++ slowtests/audio/Makefile.am 24 May 2006 21:51:45 -0000
@@ -80,3 +80,13 @@ velocity-test:
  $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy >> $(@F).tmp
  $(BSEFCOMPARE) $(srcdir)/velocity.ref $(@F).tmp --threshold 99.99
  rm -f $(@F).tmp $(@F).wav
+
+# the BEAST demo song
+FEATURE_TESTS += partymonster-test
+EXTRA_DIST += partymonster.ref
+partymonster-test:
+ $(BSE2WAV) $(top_srcdir)/library/demo/partymonster.bse $(@F).wav
+ $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 0 --avg-spectrum --spectrum --avg-energy --end-time  > $(@F).tmp
+ $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy --end-time >> $(@F).tmp
+ $(BSEFCOMPARE) $(srcdir)/partymonster.ref $(@F).tmp --threshold 99.99
+ rm -f $(@F).tmp $(@F).wav
Index: tools/bsefextract.cc
===================================================================
RCS file: /cvs/gnome/beast/tools/bsefextract.cc,v
retrieving revision 1.28
diff -u -p -r1.28 bsefextract.cc
--- tools/bsefextract.cc 21 May 2006 01:24:11 -0000 1.28
+++ tools/bsefextract.cc 24 May 2006 21:51:54 -0000
@@ -176,16 +176,25 @@ struct Feature
   const char *description;
   bool        extract_feature;      /* did the user enable this feature with --feature? */
 
+  string
+  double_to_string (double data) const
+  {
+    char *x = g_strdup_printf ("%.17g", data);
+    string s = x;
+    g_free (x);
+    return s;
+  }
+
   void print_value (const string& value_name, double data) const
   {
-    fprintf (options.output_file, "%s = %f;\n", value_name.c_str(), data);
+    fprintf (options.output_file, "%s = %s;\n", value_name.c_str(), double_to_string (data).c_str());
   }
 
   void print_vector (const string& vector_name, const vector<double>& data) const
   {
     fprintf (options.output_file, "%s[%ld] = {", vector_name.c_str(), data.size());
     for (vector<double>::const_iterator di = data.begin(); di != data.end(); di++)
-      fprintf (options.output_file, " %f", *di);
+      fprintf (options.output_file, " %s", double_to_string (*di).c_str());
     fprintf (options.output_file, " };\n");
   }
 
@@ -210,7 +219,7 @@ struct Feature
  const vector<double>& line = *mi;
 
  for (vector<double>::const_iterator li = line.begin(); li != line.end(); li++)
-  fprintf (options.output_file, " %f", *li);
+  fprintf (options.output_file, " %s", double_to_string (*li).c_str());
  fprintf (options.output_file, " }\n");
       }
     fprintf (options.output_file, "};\n");


Some comments on the patch:
 * eliminating random data in bsenoise.cc doesn't change the similarity
   score too much, but is of course necessary for 100% matches
 * davsyndrum.c: I think I found a bug here, and the fix can be
   committed right away
 * the main randomness problem really is in the davxtalstrings.c module,
   after eliminating this source of random, my compare runs were much
   more similar than before

As for the last change, I think it might be useful to increase the
precision with which bsefextract outputs its extracted features. As a
user trying to read the feature file with an editor, I dislike using the
%.17g format, because its much harder to read than %f. The other thing
that I dislike about it as a casual user is that matrix features
(--spectrum) are not aligned any more, that is, the numbers of two
different lines no longer start at the same column.

However, as a developer, I see that %f and %.17g features are so
different that changing the bsefextract output from one to another
actually breaks some existing tests. This leads me to the conclusion
that %f's implicit quantization could have the same effect in some
real world scenarios (i.e. 0.000003 vs. 0.000002 in a spectrum feature
could bring similarity below the threshold, although the "real" distance
- the "double" distance before writing the feature file - between both
extracted features might be much smaller than 0.000001).

   Cu... Stefan
--
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan
_______________________________________________
beast mailing list
beast@...
http://mail.gnome.org/mailman/listinfo/beast

[Tim Janik]

On Thu, 25 May 2006, Stefan Westerfeld wrote:

>   Hi!
>

> Index: plugins/bsenoise.cc
> ===================================================================
> RCS file: /cvs/gnome/beast/plugins/bsenoise.cc,v
> retrieving revision 1.2
> diff -u -p -r1.2 bsenoise.cc
> --- plugins/bsenoise.cc 7 Mar 2005 07:40:39 -0000 1.2
> +++ plugins/bsenoise.cc 24 May 2006 21:51:44 -0000
> @@ -53,7 +53,8 @@ class Noise : public NoiseBase {
>     {
>       g_return_if_fail (n_values <= block_size()); /* paranoid */
>
> -      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() % (noise_data->size() - n_values)]);
> +      //ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() % (noise_data->size() - n_values)]);
> +      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[0]);
>     }
>   };
> public:
> @@ -66,8 +67,12 @@ public:
>     const int N_NOISE_BLOCKS = 20;
>     noise_data.resize (block_size() * N_NOISE_BLOCKS);
>
> +    GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
> +
>     for (vector<gfloat>::iterator ni = noise_data.begin(); ni != noise_data.end(); ni++)
> -      *ni = 1.0 - rand() / (0.5 * RAND_MAX);  // FIXME: should have class noise
> +      *ni = g_rand_double_range (random_generator, -1, 1);
> +
> +    g_rand_free (random_generator);
>   }

to do something like this, we need:
- a new bse option --static-random (or similar name) to switch off real random
   numbers and use deterministic streams instead
- a new framework that allowes to open up several new random data streams
   and get data from those
- use seperate random data streams everywhere we could possibly need
   deterministic results at some point (i.e. bsenoise and davxtalstrings.c,
   possibly other modules)
- possible future randomized behaviour (e.g.l implementation of a humanizer)
   will also have to honour --static-random

>   void
>   reset1()
> Index: plugins/davsyndrum.c
> ===================================================================
> RCS file: /cvs/gnome/beast/plugins/davsyndrum.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 davsyndrum.c
> --- plugins/davsyndrum.c 16 May 2006 10:20:47 -0000 1.29
> +++ plugins/davsyndrum.c 24 May 2006 21:51:44 -0000
> @@ -259,7 +259,7 @@ dmod_process (BseModule *module,
>           /* trigger drum */
>           dmod_trigger (dmod,
>                         freq_in ? BSE_FREQ_FROM_VALUE (freq_in[i]) : dmod->params.freq,
> -                        ratio_in ? ratio_in[1] : 1.0);
> +                        ratio_in ? ratio_in[i] : 1.0); /* FIXME: <- bug? */
>           spring_vel = dmod->spring_vel;
>           env = dmod->env;
>           freq_rad = dmod->freq_rad;

could you be bothered to elabortate here?
all i can gather from your mail is "bug?" and "I think I found a bug here,
and the fix can be committed right away". i'm sorry, but for me that isn't
particularly telling, so i'd apprechiate an explanation of why you think
this is buggy (and possible investigation of source history to figure how/
if a bug was introduced here). and why you think you know what the fix would
be.

> Index: plugins/davxtalstrings.c
> ===================================================================
> RCS file: /cvs/gnome/beast/plugins/davxtalstrings.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 davxtalstrings.c
> --- plugins/davxtalstrings.c 8 May 2006 01:37:22 -0000 1.33
> +++ plugins/davxtalstrings.c 24 May 2006 21:51:44 -0000
> @@ -303,11 +303,17 @@ xmod_trigger (XtalStringsModule *xmod,
>   /* Add some snap. */
>   for (i = 0; i < xmod->size; i++)
>     xmod->string[i] = pow (xmod->string[i], xmod->tparams.snap_factor * 10.0 + 1.0);
> +
>
> +  GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
> +
>   /* Add static to displacements. */
>   for (i = 0; i < xmod->size; i++)
>     xmod->string[i] = (xmod->string[i] * (1.0F - xmod->tparams.metallic_factor) +
> -       (bse_rand_bool () ? -1.0F : 1.0F) * xmod->tparams.metallic_factor);
> +       (g_rand_boolean(random_generator) ? -1.0F : 1.0F) * xmod->tparams.metallic_factor);
> +
> +  g_rand_free (random_generator);
> +
>   /* Set velocity. */
>   for (i = 0; i < xmod->size; i++)
>     xmod->string[i] *= xmod->tparams.trigger_vel;
> Index: slowtests/audio/Makefile.am
> ===================================================================
> RCS file: /cvs/gnome/beast/slowtests/audio/Makefile.am,v
> retrieving revision 1.20
> diff -u -p -r1.20 Makefile.am
> --- slowtests/audio/Makefile.am 21 May 2006 18:04:12 -0000 1.20
> +++ slowtests/audio/Makefile.am 24 May 2006 21:51:45 -0000
> @@ -80,3 +80,13 @@ velocity-test:
> $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy >> $(@F).tmp
> $(BSEFCOMPARE) $(srcdir)/velocity.ref $(@F).tmp --threshold 99.99
> rm -f $(@F).tmp $(@F).wav
> +
> +# the BEAST demo song
> +FEATURE_TESTS += partymonster-test
> +EXTRA_DIST += partymonster.ref
> +partymonster-test:
> + $(BSE2WAV) $(top_srcdir)/library/demo/partymonster.bse $(@F).wav
> + $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 0 --avg-spectrum --spectrum --avg-energy --end-time  > $(@F).tmp
> + $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum --spectrum --avg-energy --end-time >> $(@F).tmp
> + $(BSEFCOMPARE) $(srcdir)/partymonster.ref $(@F).tmp --threshold 99.99
> + rm -f $(@F).tmp $(@F).wav

below, you're saying 99.99% is too tight to be matched in the presence of
real noise. wouldn't something like 99.8% make sense here then, until
something like --static-random is implemented?

> Index: tools/bsefextract.cc
> ===================================================================
> RCS file: /cvs/gnome/beast/tools/bsefextract.cc,v
> retrieving revision 1.28
> diff -u -p -r1.28 bsefextract.cc
> --- tools/bsefextract.cc 21 May 2006 01:24:11 -0000 1.28
> +++ tools/bsefextract.cc 24 May 2006 21:51:54 -0000
> @@ -176,16 +176,25 @@ struct Feature
>   const char *description;
>   bool        extract_feature;      /* did the user enable this feature with --feature? */
>
> +  string
> +  double_to_string (double data) const
> +  {
> +    char *x = g_strdup_printf ("%.17g", data);
> +    string s = x;
> +    g_free (x);
> +    return s;
> +  }
> +
>   void print_value (const string& value_name, double data) const
>   {
> -    fprintf (options.output_file, "%s = %f;\n", value_name.c_str(), data);
> +    fprintf (options.output_file, "%s = %s;\n", value_name.c_str(), double_to_string (data).c_str());
>   }
>
>   void print_vector (const string& vector_name, const vector<double>& data) const
>   {
>     fprintf (options.output_file, "%s[%ld] = {", vector_name.c_str(), data.size());
>     for (vector<double>::const_iterator di = data.begin(); di != data.end(); di++)
> -      fprintf (options.output_file, " %f", *di);
> +      fprintf (options.output_file, " %s", double_to_string (*di).c_str());
>     fprintf (options.output_file, " };\n");
>   }
>
> @@ -210,7 +219,7 @@ struct Feature
> const vector<double>& line = *mi;
>
> for (vector<double>::const_iterator li = line.begin(); li != line.end(); li++)
> -  fprintf (options.output_file, " %f", *li);
> +  fprintf (options.output_file, " %s", double_to_string (*li).c_str());
> fprintf (options.output_file, " }\n");
>       }
>     fprintf (options.output_file, "};\n");

i think you can commit that right away.

> Some comments on the patch:
> * eliminating random data in bsenoise.cc doesn't change the similarity
>   score too much, but is of course necessary for 100% matches
> * davsyndrum.c: I think I found a bug here, and the fix can be
>   committed right away
> * the main randomness problem really is in the davxtalstrings.c module,
>   after eliminating this source of random, my compare runs were much
>   more similar than before
>
> As for the last change, I think it might be useful to increase the
> precision with which bsefextract outputs its extracted features. As a
> user trying to read the feature file with an editor, I dislike using the
> %.17g format, because its much harder to read than %f.

i can't say i agree here. i much prefer 1.3e-69 (%g) over
   0.0000000000000000000000000000000000000000000000000000000000000000000013
(%f). and for figures with smaller exponent, %g equals %f anyway.

> The other thing
> that I dislike about it as a casual user is that matrix features
> (--spectrum) are not aligned any more, that is, the numbers of two
> different lines no longer start at the same column.

huh? but that can easily be fixed, have you tried something like %15.9g?

> However, as a developer, I see that %f and %.17g features are so
> different that changing the bsefextract output from one to another
> actually breaks some existing tests. This leads me to the conclusion
> that %f's implicit quantization could have the same effect in some
> real world scenarios (i.e. 0.000003 vs. 0.000002 in a spectrum feature
> could bring similarity below the threshold, although the "real" distance
> - the "double" distance before writing the feature file - between both
> extracted features might be much smaller than 0.000001).

%f can't be used to properly represent a double. at least not with much
less than 600 character strings. that's why our serialization functions
which can't allow for data loss are using:
   if (g_option_check (hints, "f"))      /* float hint */
     gstring_puts (gstring, g_ascii_formatd (numbuf, G_ASCII_DTOSTR_BUF_SIZE, "%.7g", sfi_value_get_real (value)));
   else
     gstring_puts (gstring, g_ascii_formatd (numbuf, G_ASCII_DTOSTR_BUF_SIZE, "%.17g", sfi_value_get_real (value)));
and why we have extra API to write out flots to data streams:
   sfi_wstore_putf();
   sfi_wstore_putd();

speaking of which, you need to change your code to use those anyway,
because currently the reference files will break in non-C locales.

>   Cu... Stefan

---
ciaoTJ
_______________________________________________
beast mailing list
beast@...
http://mail.gnome.org/mailman/listinfo/beast

[Stefan Westerfeld]

   Hi!

On Thu, May 25, 2006 at 01:58:54PM +0200, Tim Janik wrote:

> On Thu, 25 May 2006, Stefan Westerfeld wrote:
> >Index: plugins/bsenoise.cc
> >===================================================================
> >RCS file: /cvs/gnome/beast/plugins/bsenoise.cc,v
> >retrieving revision 1.2
> >diff -u -p -r1.2 bsenoise.cc
> >--- plugins/bsenoise.cc 7 Mar 2005 07:40:39 -0000 1.2
> >+++ plugins/bsenoise.cc 24 May 2006 21:51:44 -0000
> >@@ -53,7 +53,8 @@ class Noise : public NoiseBase {
> >    {
> >      g_return_if_fail (n_values <= block_size()); /* paranoid */
> >
> >-      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() %
> >(noise_data->size() - n_values)]);
> >+      //ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[rand() %
> >(noise_data->size() - n_values)]);
> >+      ostream_set (OCHANNEL_NOISE_OUT, &(*noise_data)[0]);
> >    }
> >  };
> >public:
> >@@ -66,8 +67,12 @@ public:
> >    const int N_NOISE_BLOCKS = 20;
> >    noise_data.resize (block_size() * N_NOISE_BLOCKS);
> >
> >+    GRand *random_generator = g_rand_new_with_seed (0xdeadbeaf);
> >+
> >    for (vector<gfloat>::iterator ni = noise_data.begin(); ni !=
> >    noise_data.end(); ni++)
> >-      *ni = 1.0 - rand() / (0.5 * RAND_MAX);  // FIXME: should have class
> >noise
> >+      *ni = g_rand_double_range (random_generator, -1, 1);
> >+
> >+    g_rand_free (random_generator);
> >  }
>
> to do something like this, we need:
> - a new bse option --static-random (or similar name) to switch off real
> random
>   numbers and use deterministic streams instead
> - a new framework that allowes to open up several new random data streams
>   and get data from those
> - use seperate random data streams everywhere we could possibly need
>   deterministic results at some point (i.e. bsenoise and davxtalstrings.c,
>   possibly other modules)
> - possible future randomized behaviour (e.g.l implementation of a humanizer)
>   will also have to honour --static-random

Agreed. I'd like to add that the g_random style functions are very
convenient (that is, for instance if you need a random value between -1
and 1 the code is much more readable when written with g_random than
when written without), so I'd suggest that the bse random streams should
have a similar API.

Of course I would be happy with a C++ API like

http://www.gtkmm.org/docs/glibmm-2.4/docs/reference/html/classGlib_1_1Rand.html

except of course that seeding should go away (the framework decides
whether to seed or not and how) and be replaced by a "rewind" or "reset"
function, which - if the --static-random option was given - resets the
random generator to an initial state.

> >  void
> >  reset1()
> >Index: plugins/davsyndrum.c
> >===================================================================
> >RCS file: /cvs/gnome/beast/plugins/davsyndrum.c,v
> >retrieving revision 1.29
> >diff -u -p -r1.29 davsyndrum.c
> >--- plugins/davsyndrum.c 16 May 2006 10:20:47 -0000 1.29
> >+++ plugins/davsyndrum.c 24 May 2006 21:51:44 -0000
> >@@ -259,7 +259,7 @@ dmod_process (BseModule *module,
> >          /* trigger drum */
> >          dmod_trigger (dmod,
> >                        freq_in ? BSE_FREQ_FROM_VALUE (freq_in[i]) :
> >                        dmod->params.freq,
> >-                        ratio_in ? ratio_in[1] : 1.0);
> >+                        ratio_in ? ratio_in[i] : 1.0); /* FIXME: <- bug?
> >*/
> >          spring_vel = dmod->spring_vel;
> >          env = dmod->env;
> >          freq_rad = dmod->freq_rad;
>
> could you be bothered to elabortate here?
> all i can gather from your mail is "bug?" and "I think I found a bug here,
> and the fix can be committed right away". i'm sorry, but for me that isn't
> particularly telling, so i'd apprechiate an explanation of why you think
> this is buggy (and possible investigation of source history to figure how/
> if a bug was introduced here). and why you think you know what the fix would
> be.

Well, the point is that we're receiving a trigger event here, and the
event is at position "i" in the stream. However, the code in the CVS
doesn't use ratio_in at position "i" to determine the strength of the
trigger event, but ratio_in at position "1". Thus it doesn't use the
right ratio associated with the trigger event (and possibly even
accesses memory beyond the buffer size, although accessing the "1"
element shouldn't be particularily bad).

When it comes to my patch, what is confusing is maybe that I left the
FIXME comment there. With "ratio_in[i]" (instead of "ratio_in[1]"), as
of after applying the patch, there is no problem.

> >Index: slowtests/audio/Makefile.am
> >===================================================================
> >RCS file: /cvs/gnome/beast/slowtests/audio/Makefile.am,v
> >retrieving revision 1.20
> >diff -u -p -r1.20 Makefile.am
> >--- slowtests/audio/Makefile.am 21 May 2006 18:04:12 -0000 1.20
> >+++ slowtests/audio/Makefile.am 24 May 2006 21:51:45 -0000
> >@@ -80,3 +80,13 @@ velocity-test:
> > $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum
> > --spectrum --avg-energy >> $(@F).tmp
> > $(BSEFCOMPARE) $(srcdir)/velocity.ref $(@F).tmp --threshold 99.99
> > rm -f $(@F).tmp $(@F).wav
> >+
> >+# the BEAST demo song
> >+FEATURE_TESTS += partymonster-test
> >+EXTRA_DIST += partymonster.ref
> >+partymonster-test:
> >+ $(BSE2WAV) $(top_srcdir)/library/demo/partymonster.bse $(@F).wav
> >+ $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 0 --avg-spectrum
> >--spectrum --avg-energy --end-time  > $(@F).tmp
> >+ $(BSEFEXTRACT) $(@F).wav --cut-zeros --channel 1 --avg-spectrum
> >--spectrum --avg-energy --end-time >> $(@F).tmp
> >+ $(BSEFCOMPARE) $(srcdir)/partymonster.ref $(@F).tmp --threshold 99.99
> >+ rm -f $(@F).tmp $(@F).wav
>
> below, you're saying 99.99% is too tight to be matched in the presence of
> real noise. wouldn't something like 99.8% make sense here then, until
> something like --static-random is implemented?

Yes, it should work. I took 99.9% now, which should pass. Of course,
since we're relying on random data here, I can not guarantee that the
test will always pass, as for some obscure random sequence it may sound
more different than for those sequences I tested it with. But three test
runs give me:

average similarity rating: 99.951312% => good match.
average similarity rating: 99.952625% => good match.
average similarity rating: 99.955167% => good match.

Which doesn't look like there is much danger of this average value
dropping below 99.9%.

> >Index: tools/bsefextract.cc
> >===================================================================
> >RCS file: /cvs/gnome/beast/tools/bsefextract.cc,v
> >retrieving revision 1.28
> >diff -u -p -r1.28 bsefextract.cc
> >--- tools/bsefextract.cc 21 May 2006 01:24:11 -0000 1.28
> >+++ tools/bsefextract.cc 24 May 2006 21:51:54 -0000
> >@@ -176,16 +176,25 @@ struct Feature
> >  const char *description;
> >  bool        extract_feature;      /* did the user enable this feature
> >  with --feature? */
> >
> >+  string
> >+  double_to_string (double data) const
> >+  {
> >+    char *x = g_strdup_printf ("%.17g", data);
> >+    string s = x;
> >+    g_free (x);
> >+    return s;
> >+  }
> >+
> >  void print_value (const string& value_name, double data) const
> >  {
> >-    fprintf (options.output_file, "%s = %f;\n", value_name.c_str(), data);
> >+    fprintf (options.output_file, "%s = %s;\n", value_name.c_str(),
> >double_to_string (data).c_str());
> >  }
> >
> >  void print_vector (const string& vector_name, const vector<double>&
> >  data) const
> >  {
> >    fprintf (options.output_file, "%s[%ld] = {", vector_name.c_str(),
> >    data.size());
> >    for (vector<double>::const_iterator di = data.begin(); di !=
> >    data.end(); di++)
> >-      fprintf (options.output_file, " %f", *di);
> >+      fprintf (options.output_file, " %s", double_to_string
> >(*di).c_str());
> >    fprintf (options.output_file, " };\n");
> >  }
> >
> >@@ -210,7 +219,7 @@ struct Feature
> > const vector<double>& line = *mi;
> >
> > for (vector<double>::const_iterator li = line.begin(); li !=
> > line.end(); li++)
> >-  fprintf (options.output_file, " %f", *li);
> >+  fprintf (options.output_file, " %s", double_to_string
> >(*li).c_str());
> > fprintf (options.output_file, " }\n");
> >      }
> >    fprintf (options.output_file, "};\n");
>
> i think you can commit that right away.

Ok, in CVS now (with modifications, see below).

> >Some comments on the patch:
> >* eliminating random data in bsenoise.cc doesn't change the similarity
> >  score too much, but is of course necessary for 100% matches
> >* davsyndrum.c: I think I found a bug here, and the fix can be
> >  committed right away
> >* the main randomness problem really is in the davxtalstrings.c module,
> >  after eliminating this source of random, my compare runs were much
> >  more similar than before
> >
> >As for the last change, I think it might be useful to increase the
> >precision with which bsefextract outputs its extracted features. As a
> >user trying to read the feature file with an editor, I dislike using the
> >%.17g format, because its much harder to read than %f.
>
> i can't say i agree here. i much prefer 1.3e-69 (%g) over
>   0.0000000000000000000000000000000000000000000000000000000000000000000013
> (%f). and for figures with smaller exponent, %g equals %f anyway.

As for what bsefextract and bsefcompare do right now, such a small
number would usually be meaningless anyway; in a frequency spectrum for
instance, a frequency with this amplitude couldn't be percieved by
anybody.

So what I am saying is that for your number storing 0.0 would be
perfectly all right. But yes, planning for any possible future use, I
can't guarantee that we won't have doubles with such exponents which do
have a meaning (although right now I can't think of any), so %g is
probably superior.

> >The other thing
> >that I dislike about it as a casual user is that matrix features
> >(--spectrum) are not aligned any more, that is, the numbers of two
> >different lines no longer start at the same column.
>
> huh? but that can easily be fixed, have you tried something like %15.9g?

Ok, I use it now. Actually I use %-15.9g for the matrix and vector
formats and %.9g for the normal format; I think the files look somewhat
readable, although of course the lines became much longer.

> >However, as a developer, I see that %f and %.17g features are so
> >different that changing the bsefextract output from one to another
> >actually breaks some existing tests. This leads me to the conclusion
> >that %f's implicit quantization could have the same effect in some
> >real world scenarios (i.e. 0.000003 vs. 0.000002 in a spectrum feature
> >could bring similarity below the threshold, although the "real" distance
> >- the "double" distance before writing the feature file - between both
> >extracted features might be much smaller than 0.000001).
>
> %f can't be used to properly represent a double. at least not with much
> less than 600 character strings. that's why our serialization functions
> which can't allow for data loss are using:
>   if (g_option_check (hints, "f"))      /* float hint */
>     gstring_puts (gstring, g_ascii_formatd (numbuf,
>     G_ASCII_DTOSTR_BUF_SIZE, "%.7g", sfi_value_get_real (value)));
>   else
>     gstring_puts (gstring, g_ascii_formatd (numbuf,
>     G_ASCII_DTOSTR_BUF_SIZE, "%.17g", sfi_value_get_real (value)));
> and why we have extra API to write out flots to data streams:
>   sfi_wstore_putf();
>   sfi_wstore_putd();
>
> speaking of which, you need to change your code to use those anyway,
> because currently the reference files will break in non-C locales.

Ok, I am using g_ascii_format_d now, as you suggested.

   Cu... Stefan
--
Stefan Westerfeld, Hamburg/Germany, http://space.twc.de/~stefan
_______________________________________________
beast mailing list
beast@...
http://mail.gnome.org/mailman/listinfo/beast