FREQUENCIES: Added new test for bug which causes a crash
[pspp] / src / language / stats / frequencies.q
index 1973bd94d252b157ac343da954c1f1341561cd44..8a8ac6c4bd64b4633099c5db1b5f53e69774cb38 100644 (file)
@@ -1,5 +1,5 @@
 /* PSPP - a program for statistical analysis.
-   Copyright (C) 1997-9, 2000, 2007, 2009, 2010 Free Software Foundation, Inc.
+   Copyright (C) 1997-9, 2000, 2007, 2009, 2010, 2011 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -23,9 +23,9 @@
 #include "data/case.h"
 #include "data/casegrouper.h"
 #include "data/casereader.h"
+#include "data/dataset.h"
 #include "data/dictionary.h"
 #include "data/format.h"
-#include "data/procedure.h"
 #include "data/settings.h"
 #include "data/value-labels.h"
 #include "data/variable.h"
@@ -43,6 +43,8 @@
 #include "libpspp/str.h"
 #include "math/histogram.h"
 #include "math/moments.h"
+#include "math/chart-geometry.h"
+
 #include "output/chart-item.h"
 #include "output/charts/piechart.h"
 #include "output/charts/plot-hist.h"
@@ -125,17 +127,9 @@ struct percentile
 {
   double p;        /* the %ile to be calculated */
   double value;    /* the %ile's value */
-  double x1;       /* The datum value <= the percentile */
-  double x2;       /* The datum value >= the percentile */
-  int flag;
-  int flag2;       /* Set to 1 if this percentile value has been found */
   bool show;       /* True to show this percentile in the statistics box. */
 };
 
-
-static struct percentile *percentiles;
-static int n_percentiles, n_show_percentiles;
-
 /* Groups of statistics. */
 #define BI          BIT_INDEX
 #define FRQ_DEFAULT                                                    \
@@ -147,10 +141,6 @@ static int n_percentiles, n_show_percentiles;
         | BI(FRQ_SKEW) | BI(FRQ_SESKEW) | BI(FRQ_RANGE)        \
         | BI(FRQ_RANGE) | BI(FRQ_MODE) | BI(FRQ_MEDIAN))
 
-/* Statistics; number of statistics. */
-static unsigned long stats;
-static int n_stats;
-
 struct frq_chart
   {
     double x_min;               /* X axis minimum value. */
@@ -165,12 +155,6 @@ struct frq_chart
     bool include_missing;       /* Whether to include missing values. */
   };
 
-/* Histogram and pie chart settings. */
-static struct frq_chart hist, pie;
-
-/* Parsed command. */
-static struct cmd_frequencies cmd;
-
 /* Frequency tables. */
 
 /* Entire frequency table. */
@@ -206,7 +190,6 @@ struct var_freqs
 
     /* Variable attributes. */
     int width;
-    struct fmt_spec print;
   };
 
 struct frq_proc
@@ -215,88 +198,116 @@ struct frq_proc
 
     struct var_freqs *vars;
     size_t n_vars;
+
+    /* Percentiles to calculate and possibly display. */
+    struct percentile *percentiles;
+    int n_percentiles, n_show_percentiles;
+
+    /* Frequency table display. */
+    int max_categories;         /* Maximum categories to show. */
+    int sort;                   /* FRQ_AVALUE or FRQ_DVALUE
+                                   or FRQ_ACOUNT or FRQ_DCOUNT. */
+
+    /* Statistics; number of statistics. */
+    unsigned long stats;
+    int n_stats;
+
+    /* Histogram and pie chart settings. */
+    struct frq_chart *hist, *pie;
   };
 
-static void determine_charts (void);
+static void determine_charts (struct frq_proc *,
+                              const struct cmd_frequencies *);
 
-static void calc_stats (const struct var_freqs *v, double d[FRQ_N_STATS]);
+static void calc_stats (const struct var_freqs *, double d[FRQ_N_STATS]);
+static void calc_percentiles (const struct frq_proc *,
+                              const struct var_freqs *);
 
 static void precalc (struct frq_proc *, struct casereader *, struct dataset *);
 static void calc (struct frq_proc *, const struct ccase *,
                   const struct dataset *);
 static void postcalc (struct frq_proc *, const struct dataset *);
 
-static void postprocess_freq_tab (struct var_freqs *);
+static void postprocess_freq_tab (const struct frq_proc *, struct var_freqs *);
 static void dump_freq_table (const struct var_freqs *,
                              const struct variable *weight_var);
-static void dump_statistics (const struct var_freqs *,
+static void dump_statistics (const struct frq_proc *, const struct var_freqs *,
                              const struct variable *weight_var);
 static void cleanup_freq_tab (struct var_freqs *);
 
-static algo_compare_func compare_value_numeric_a, compare_value_alpha_a;
-static algo_compare_func compare_value_numeric_d, compare_value_alpha_d;
-static algo_compare_func compare_freq_numeric_a, compare_freq_alpha_a;
-static algo_compare_func compare_freq_numeric_d, compare_freq_alpha_d;
-
-static void add_percentile (struct frq_proc *, double x, bool show);
-
-static void do_piechart(const struct variable *var,
-                       const struct freq_tab *frq_tab);
-
-struct histogram *
-freq_tab_to_hist(const struct freq_tab *ft, const struct variable *var);
+static void add_percentile (struct frq_proc *, double x, bool show,
+                            size_t *allocated_percentiles);
 
+static void do_piechart(const struct frq_chart *, const struct variable *,
+                       const struct freq_tab *);
 
+struct histogram *freq_tab_to_hist(const struct frq_proc *,
+                                   const struct freq_tab *,
+                                   const struct variable *);
 \f
 /* Parser and outline. */
 
 int
 cmd_frequencies (struct lexer *lexer, struct dataset *ds)
 {
+  struct cmd_frequencies cmd;
   struct frq_proc frq;
   struct casegrouper *grouper;
   struct casereader *input, *group;
+  size_t allocated_percentiles;
   bool ok;
   int i;
 
-  n_percentiles = 0;
-  n_show_percentiles = 0;
-  percentiles = NULL;
-
   frq.pool = pool_create ();
+
   frq.vars = NULL;
   frq.n_vars = 0;
 
+  frq.percentiles = NULL;
+  frq.n_percentiles = 0;
+  frq.n_show_percentiles = 0;
+
+  frq.hist = NULL;
+  frq.pie = NULL;
+
+  allocated_percentiles = 0;
+
   if (!parse_frequencies (lexer, ds, &cmd, &frq))
     {
       pool_destroy (frq.pool);
       return CMD_FAILURE;
     }
 
+  /* Figure out when to show frequency tables. */
+  frq.max_categories = (cmd.table == FRQ_NOTABLE ? -1
+                        : cmd.table == FRQ_TABLE ? INT_MAX
+                        : cmd.limit);
+  frq.sort = cmd.sort;
+
   /* Figure out statistics to calculate. */
-  stats = 0;
+  frq.stats = 0;
   if (cmd.a_statistics[FRQ_ST_DEFAULT] || !cmd.sbc_statistics)
-    stats |= FRQ_DEFAULT;
+    frq.stats |= FRQ_DEFAULT;
   if (cmd.a_statistics[FRQ_ST_ALL])
-    stats |= FRQ_ALL;
+    frq.stats |= FRQ_ALL;
   if (cmd.sort != FRQ_AVALUE && cmd.sort != FRQ_DVALUE)
-    stats &= ~BIT_INDEX (FRQ_MEDIAN);
+    frq.stats &= ~BIT_INDEX (FRQ_MEDIAN);
   for (i = 0; i < FRQ_N_STATS; i++)
     if (cmd.a_statistics[st_name[i].st_indx])
-      stats |= BIT_INDEX (i);
-  if (stats & FRQ_KURT)
-    stats |= BIT_INDEX (FRQ_SEKURT);
-  if (stats & FRQ_SKEW)
-    stats |= BIT_INDEX (FRQ_SESKEW);
+      frq.stats |= BIT_INDEX (i);
+  if (frq.stats & FRQ_KURT)
+    frq.stats |= BIT_INDEX (FRQ_SEKURT);
+  if (frq.stats & FRQ_SKEW)
+    frq.stats |= BIT_INDEX (FRQ_SESKEW);
 
   /* Calculate n_stats. */
-  n_stats = 0;
+  frq.n_stats = 0;
   for (i = 0; i < FRQ_N_STATS; i++)
-    if ((stats & BIT_INDEX (i)))
-      n_stats++;
+    if ((frq.stats & BIT_INDEX (i)))
+      frq.n_stats++;
 
   /* Charting. */
-  determine_charts ();
+  determine_charts (&frq, &cmd);
   if (cmd.sbc_histogram || cmd.sbc_piechart || cmd.sbc_ntiles)
     cmd.sort = FRQ_AVALUE;
 
@@ -309,7 +320,7 @@ cmd_frequencies (struct lexer *lexer, struct dataset *ds)
          subc_list_double *ptl_list = &cmd.dl_percentiles[i];
          for ( pl = 0 ; pl < subc_list_double_count(ptl_list); ++pl)
             add_percentile (&frq, subc_list_double_at(ptl_list, pl) / 100.0,
-                            true);
+                            true, &allocated_percentiles);
        }
     }
   if ( cmd.sbc_ntiles )
@@ -318,21 +329,22 @@ cmd_frequencies (struct lexer *lexer, struct dataset *ds)
        {
          int j;
          for (j = 0; j <= cmd.n_ntiles[i]; ++j )
-            add_percentile (&frq, j / (double) cmd.n_ntiles[i], true);
+            add_percentile (&frq, j / (double) cmd.n_ntiles[i], true,
+                            &allocated_percentiles);
        }
     }
-  if (stats & BIT_INDEX (FRQ_MEDIAN))
+  if (frq.stats & BIT_INDEX (FRQ_MEDIAN))
     {
       /* Treat the median as the 50% percentile.
          We output it in the percentiles table as "50 (Median)." */
-      add_percentile (&frq, 0.5, true);
-      stats &= ~BIT_INDEX (FRQ_MEDIAN);
-      n_stats--;
+      add_percentile (&frq, 0.5, true, &allocated_percentiles);
+      frq.stats &= ~BIT_INDEX (FRQ_MEDIAN);
+      frq.n_stats--;
     }
   if (cmd.sbc_histogram)
     {
-      add_percentile (&frq, 0.25, false);
-      add_percentile (&frq, 0.75, false);
+      add_percentile (&frq, 0.25, false, &allocated_percentiles);
+      add_percentile (&frq, 0.75, false, &allocated_percentiles);
     }
 
   /* Do it! */
@@ -356,53 +368,69 @@ cmd_frequencies (struct lexer *lexer, struct dataset *ds)
 
   pool_destroy (frq.pool);
   free (frq.vars);
+  free (frq.percentiles);
+  free (frq.hist);
+  free (frq.pie);
 
   return ok ? CMD_SUCCESS : CMD_CASCADING_FAILURE;
 }
 
 /* Figure out which charts the user requested.  */
 static void
-determine_charts (void)
+determine_charts (struct frq_proc *frq, const struct cmd_frequencies *cmd)
 {
-  if (cmd.sbc_barchart)
+  if (cmd->sbc_barchart)
     msg (SW, _("Bar charts are not implemented."));
 
-  if (cmd.sbc_histogram)
+  if (cmd->sbc_histogram)
     {
-      hist.x_min = cmd.hi_min;
-      hist.x_max = cmd.hi_max;
-      hist.y_scale = cmd.hi_scale;
-      hist.y_max = cmd.hi_scale == FRQ_FREQ ? cmd.hi_freq : cmd.hi_pcnt;
-      hist.draw_normal = cmd.hi_norm != FRQ_NONORMAL;
-      hist.include_missing = false;
-
-      if (hist.x_min != SYSMIS && hist.x_max != SYSMIS
-          && hist.x_min >= hist.x_max)
+      struct frq_chart *hist;
+
+      hist = frq->hist = xmalloc (sizeof *frq->hist);
+      hist->x_min = cmd->hi_min;
+      hist->x_max = cmd->hi_max;
+      hist->y_scale = cmd->hi_scale;
+      hist->y_max = cmd->hi_scale == FRQ_FREQ ? cmd->hi_freq : cmd->hi_pcnt;
+      hist->draw_normal = cmd->hi_norm != FRQ_NONORMAL;
+      hist->include_missing = false;
+
+      if (hist->x_min != SYSMIS && hist->x_max != SYSMIS
+          && hist->x_min >= hist->x_max)
         {
-          msg (SE, _("MAX for histogram must be greater than or equal to MIN, "
-                     "but MIN was specified as %.15g and MAX as %.15g.  "
-                     "MIN and MAX will be ignored."), hist.x_min, hist.x_max);
-          hist.x_min = hist.x_max = SYSMIS;
+          msg (SE, _("%s for histogram must be greater than or equal to %s, "
+                     "but %s was specified as %.15g and %s as %.15g.  "
+                     "%s and %s will be ignored."),
+              "MAX", "MIN", 
+              "MIN", hist->x_min, 
+              "MAX", hist->x_max,
+              "MIN", "MAX");
+          hist->x_min = hist->x_max = SYSMIS;
         }
     }
 
-  if (cmd.sbc_piechart)
+  if (cmd->sbc_piechart)
     {
-      pie.x_min = cmd.pie_min;
-      pie.x_max = cmd.pie_max;
-      pie.y_scale = cmd.pie_scale;
-      pie.include_missing = cmd.pie_missing == FRQ_MISSING;
+      struct frq_chart *pie;
+
+      pie = frq->pie = xmalloc (sizeof *frq->pie);
+      pie->x_min = cmd->pie_min;
+      pie->x_max = cmd->pie_max;
+      pie->y_scale = cmd->pie_scale;
+      pie->include_missing = cmd->pie_missing == FRQ_MISSING;
 
-      if (pie.x_min != SYSMIS && pie.x_max != SYSMIS
-          && pie.x_min >= pie.x_max)
+      if (pie->x_min != SYSMIS && pie->x_max != SYSMIS
+          && pie->x_min >= pie->x_max)
         {
-          msg (SE, _("MAX for pie chart must be greater than or equal to MIN, "
-                     "but MIN was specified as %.15g and MAX as %.15g.  "
-                     "MIN and MAX will be ignored."), pie.x_min, pie.x_max);
-          pie.x_min = pie.x_max = SYSMIS;
+          msg (SE, _("%s for pie chart must be greater than or equal to %s, "
+                     "but %s was specified as %.15g and %s as %.15g.  "
+                     "%s and %s will be ignored."), 
+              "MAX", "MIN", 
+              "MIN", pie->x_min,
+              "MAX", pie->x_max,
+              "MIN", "MAX");
+          pie->x_min = pie->x_max = SYSMIS;
         }
     }
-
 }
 
 /* Add data from case C to the frequency table. */
@@ -458,69 +486,47 @@ postcalc (struct frq_proc *frq, const struct dataset *ds)
   for (i = 0; i < frq->n_vars; i++)
     {
       struct var_freqs *vf = &frq->vars[i];
-      int n_categories;
 
-      postprocess_freq_tab (vf);
+      postprocess_freq_tab (frq, vf);
 
       /* Frequencies tables. */
-      n_categories = vf->tab.n_valid + vf->tab.n_missing;
-      if  (cmd.table == FRQ_TABLE
-           || (cmd.table == FRQ_LIMIT && n_categories <= cmd.limit))
+      if (vf->tab.n_valid + vf->tab.n_missing <= frq->max_categories)
         dump_freq_table (vf, wv);
 
       /* Statistics. */
-      if (n_stats)
-       dump_statistics (vf, wv);
+      if (frq->n_stats)
+       dump_statistics (frq, vf, wv);
 
-      if (cmd.sbc_histogram && var_is_numeric (vf->var) && vf->tab.n_valid > 0)
+      if (frq->hist && var_is_numeric (vf->var) && vf->tab.n_valid > 0)
        {
          double d[FRQ_N_STATS];
          struct histogram *histogram;
 
          calc_stats (vf, d);
 
-         histogram = freq_tab_to_hist (&vf->tab, vf->var);
+         histogram = freq_tab_to_hist (frq, &vf->tab, vf->var);
 
-          chart_item_submit (histogram_chart_create (
+         if ( histogram)
+           {
+             chart_item_submit (histogram_chart_create (
                                histogram->gsl_hist, var_to_string(vf->var),
                                vf->tab.valid_cases,
                                d[FRQ_MEAN],
                                d[FRQ_STDDEV],
-                               hist.draw_normal));
+                               frq->hist->draw_normal));
 
-         statistic_destroy (&histogram->parent);
+             statistic_destroy (&histogram->parent);
+           }
        }
 
-      if (cmd.sbc_piechart)
-        do_piechart(vf->var, &vf->tab);
+      if (frq->pie)
+        do_piechart(frq->pie, vf->var, &vf->tab);
 
       cleanup_freq_tab (vf);
 
     }
 }
 
-/* Returns the comparison function that should be used for
-   sorting a frequency table by FRQ_SORT using VAL_TYPE
-   values. */
-static algo_compare_func *
-get_freq_comparator (int frq_sort, enum val_type val_type)
-{
-  bool is_numeric = val_type == VAL_NUMERIC;
-  switch (frq_sort)
-    {
-    case FRQ_AVALUE:
-      return is_numeric ? compare_value_numeric_a : compare_value_alpha_a;
-    case FRQ_DVALUE:
-      return is_numeric ? compare_value_numeric_d : compare_value_alpha_d;
-    case FRQ_AFREQ:
-      return is_numeric ? compare_freq_numeric_a : compare_freq_alpha_a;
-    case FRQ_DFREQ:
-      return is_numeric ? compare_freq_numeric_d : compare_freq_alpha_d;
-    default:
-      NOT_REACHED ();
-    }
-}
-
 /* Returns true iff the value in struct freq F is non-missing
    for variable V. */
 static bool
@@ -532,12 +538,39 @@ not_missing (const void *f_, const void *v_)
   return !var_is_value_missing (v, &f->value, MV_ANY);
 }
 
+struct freq_compare_aux
+  {
+    bool by_freq;
+    bool ascending_freq;
+
+    int width;
+    bool ascending_value;
+  };
+
+static int
+compare_freq (const void *a_, const void *b_, const void *aux_)
+{
+  const struct freq_compare_aux *aux = aux_;
+  const struct freq *a = a_;
+  const struct freq *b = b_;
+
+  if (aux->by_freq && a->count != b->count)
+    {
+      int cmp = a->count > b->count ? 1 : -1;
+      return aux->ascending_freq ? cmp : -cmp;
+    }
+  else
+    {
+      int cmp = value_compare_3way (&a->value, &b->value, aux->width);
+      return aux->ascending_value ? cmp : -cmp;
+    }
+}
 /* Summarizes the frequency table data for variable V. */
 static void
-postprocess_freq_tab (struct var_freqs *vf)
+postprocess_freq_tab (const struct frq_proc *frq, struct var_freqs *vf)
 {
   struct freq_tab *ft = &vf->tab;
-  algo_compare_func *compare;
+  struct freq_compare_aux aux;
   size_t count;
   struct freq *freqs, *f;
   size_t i;
@@ -553,9 +586,12 @@ postprocess_freq_tab (struct var_freqs *vf)
   ft->n_missing = count - ft->n_valid;
 
   /* Sort data. */
-  compare = get_freq_comparator (cmd.sort, var_get_type (vf->var));
-  sort (ft->valid, ft->n_valid, sizeof *ft->valid, compare, vf);
-  sort (ft->missing, ft->n_missing, sizeof *ft->missing, compare, vf);
+  aux.by_freq = frq->sort == FRQ_AFREQ || frq->sort == FRQ_DFREQ;
+  aux.ascending_freq = frq->sort != FRQ_DFREQ;
+  aux.width = vf->width;
+  aux.ascending_value = frq->sort != FRQ_DVALUE;
+  sort (ft->valid, ft->n_valid, sizeof *ft->valid, compare_freq, &aux);
+  sort (ft->missing, ft->n_missing, sizeof *ft->missing, compare_freq, &aux);
 
   /* Summary statistics. */
   ft->valid_cases = 0.0;
@@ -593,10 +629,10 @@ frq_custom_variables (struct lexer *lexer, struct dataset *ds,
   size_t n_vars;
   size_t i;
 
-  lex_match (lexer, '=');
+  lex_match (lexer, T_EQUALS);
   if (lex_token (lexer) != T_ALL
       && (lex_token (lexer) != T_ID
-          || dict_lookup_var (dataset_dict (ds), lex_tokid (lexer)) == NULL))
+          || dict_lookup_var (dataset_dict (ds), lex_tokcstr (lexer)) == NULL))
     return 2;
 
   /* Get list of current variables, to avoid duplicates. */
@@ -621,7 +657,6 @@ frq_custom_variables (struct lexer *lexer, struct dataset *ds,
       vf->n_groups = 0;
       vf->groups = NULL;
       vf->width = var_get_width (var);
-      vf->print = *var_get_print_format (var);
     }
   frq->n_vars = n_vars;
 
@@ -637,8 +672,9 @@ frq_custom_grouped (struct lexer *lexer, struct dataset *ds, struct cmd_frequenc
 {
   struct frq_proc *frq = frq_;
 
-  lex_match (lexer, '=');
-  if ((lex_token (lexer) == T_ID && dict_lookup_var (dataset_dict (ds), lex_tokid (lexer)) != NULL)
+  lex_match (lexer, T_EQUALS);
+  if ((lex_token (lexer) == T_ID
+       && dict_lookup_var (dataset_dict (ds), lex_tokcstr (lexer)) != NULL)
       || lex_token (lexer) == T_ID)
     for (;;)
       {
@@ -655,7 +691,7 @@ frq_custom_grouped (struct lexer *lexer, struct dataset *ds, struct cmd_frequenc
        if (!parse_variables_const (lexer, dataset_dict (ds), &v, &n,
                               PV_NO_DUPLICATE | PV_NUMERIC))
          return 0;
-       if (lex_match (lexer, '('))
+       if (lex_match (lexer, T_LPAREN))
          {
            nl = ml = 0;
            dl = NULL;
@@ -668,14 +704,14 @@ frq_custom_grouped (struct lexer *lexer, struct dataset *ds, struct cmd_frequenc
                  }
                dl[nl++] = lex_tokval (lexer);
                lex_get (lexer);
-               lex_match (lexer, ',');
+               lex_match (lexer, T_COMMA);
              }
            /* Note that nl might still be 0 and dl might still be
               NULL.  That's okay. */
-           if (!lex_match (lexer, ')'))
+           if (!lex_match (lexer, T_RPAREN))
              {
                free (v);
-               msg (SE, _("`)' expected after GROUPED interval list."));
+                lex_error_expecting (lexer, "`)'", NULL_SENTINEL);
                return 0;
              }
          }
@@ -696,7 +732,7 @@ frq_custom_grouped (struct lexer *lexer, struct dataset *ds, struct cmd_frequenc
                   {
                     if (vf->groups != NULL)
                       msg (SE, _("Variables %s specified multiple times on "
-                                 "GROUPED subcommand."), var_get_name (v[i]));
+                                 "%s subcommand."), var_get_name (v[i]), "GROUPED");
                     else
                       {
                         vf->n_groups = nl;
@@ -705,21 +741,30 @@ frq_custom_grouped (struct lexer *lexer, struct dataset *ds, struct cmd_frequenc
                     goto found;
                   }
               }
-            msg (SE, _("Variables %s specified on GROUPED but not on "
-                       "VARIABLES."), var_get_name (v[i]));
+            msg (SE, _("Variables %s specified on %s but not on "
+                       "%s."), var_get_name (v[i]), "GROUPED", "VARIABLES");
 
           found:;
           }
 
        free (v);
-       if (!lex_match (lexer, '/'))
-         break;
-       if ((lex_token (lexer) != T_ID || dict_lookup_var (dataset_dict (ds), lex_tokid (lexer)) != NULL)
-            && lex_token (lexer) != T_ALL)
-         {
-           lex_put_back (lexer, '/');
-           break;
-         }
+        if (lex_token (lexer) != T_SLASH)
+          break;
+
+        if ((lex_next_token (lexer, 1) == T_ID
+             && dict_lookup_var (dataset_dict (ds),
+                                 lex_next_tokcstr (lexer, 1)))
+            || lex_next_token (lexer, 1) == T_ALL)
+          {
+            /* The token after the slash is a variable name.  Keep parsing. */
+            lex_get (lexer);
+          }
+        else
+          {
+            /* The token after the slash must be the start of a new
+               subcommand.  Let the caller see the slash. */
+            break;
+          }
       }
 
   return 1;
@@ -729,158 +774,44 @@ frq_custom_grouped (struct lexer *lexer, struct dataset *ds, struct cmd_frequenc
    order.  If SHOW is true, the percentile will be shown in the statistics
    box, otherwise it will be hidden. */
 static void
-add_percentile (struct frq_proc *frq, double x, bool show)
+add_percentile (struct frq_proc *frq, double x, bool show,
+                size_t *allocated_percentiles)
 {
   int i;
 
-  for (i = 0; i < n_percentiles; i++)
+  /* Do nothing if it's already in the list */
+  for (i = 0; i < frq->n_percentiles; i++)
     {
-      /* Do nothing if it's already in the list */
-      if ( fabs(x - percentiles[i].p) < DBL_EPSILON )
+      struct percentile *pc = &frq->percentiles[i];
+
+      if ( fabs(x - pc->p) < DBL_EPSILON )
         {
-          if (show && !percentiles[i].show)
+          if (show && !pc->show)
             {
-              n_show_percentiles++;
-              percentiles[i].show = true;
+              frq->n_show_percentiles++;
+              pc->show = true;
             }
           return;
         }
 
-      if (x < percentiles[i].p)
+      if (x < pc->p)
        break;
     }
 
-  if (i >= n_percentiles || x != percentiles[i].p)
-    {
-      percentiles = pool_nrealloc (frq->pool, percentiles,
-                                   n_percentiles + 1, sizeof *percentiles);
-      insert_element (percentiles, n_percentiles, sizeof *percentiles, i);
-      percentiles[i].p = x;
-      percentiles[i].show = show;
-      n_percentiles++;
-      if (show)
-        n_show_percentiles++;
-    }
+  if (frq->n_percentiles >= *allocated_percentiles)
+    frq->percentiles = x2nrealloc (frq->percentiles, allocated_percentiles,
+                                   sizeof *frq->percentiles);
+  insert_element (frq->percentiles, frq->n_percentiles,
+                  sizeof *frq->percentiles, i);
+  frq->percentiles[i].p = x;
+  frq->percentiles[i].show = show;
+  frq->n_percentiles++;
+  if (show)
+    frq->n_show_percentiles++;
 }
 
 /* Comparison functions. */
 
-/* Ascending numeric compare of values. */
-static int
-compare_value_numeric_a (const void *a_, const void *b_,
-                         const void *vf_ UNUSED)
-{
-  const struct freq *a = a_;
-  const struct freq *b = b_;
-
-  if (a->value.f > b->value.f)
-    return 1;
-  else if (a->value.f < b->value.f)
-    return -1;
-  else
-    return 0;
-}
-
-/* Ascending string compare of values. */
-static int
-compare_value_alpha_a (const void *a_, const void *b_, const void *vf_)
-{
-  const struct freq *a = a_;
-  const struct freq *b = b_;
-  const struct var_freqs *vf = vf_;
-
-  return value_compare_3way (&a->value, &b->value, vf->width);
-}
-
-/* Descending numeric compare of values. */
-static int
-compare_value_numeric_d (const void *a, const void *b, const void *vf_ UNUSED)
-{
-  return -compare_value_numeric_a (a, b, vf_);
-}
-
-/* Descending string compare of values. */
-static int
-compare_value_alpha_d (const void *a, const void *b, const void *vf_)
-{
-  return -compare_value_alpha_a (a, b, vf_);
-}
-
-/* Ascending numeric compare of frequency;
-   secondary key on ascending numeric value. */
-static int
-compare_freq_numeric_a (const void *a_, const void *b_, const void *vf_ UNUSED)
-{
-  const struct freq *a = a_;
-  const struct freq *b = b_;
-
-  if (a->count > b->count)
-    return 1;
-  else if (a->count < b->count)
-    return -1;
-
-  if (a->value.f > b->value.f)
-    return 1;
-  else if (a->value.f < b->value.f)
-    return -1;
-  else
-    return 0;
-}
-
-/* Ascending numeric compare of frequency;
-   secondary key on ascending string value. */
-static int
-compare_freq_alpha_a (const void *a_, const void *b_, const void *vf_)
-{
-  const struct freq *a = a_;
-  const struct freq *b = b_;
-  const struct var_freqs *vf = vf_;
-
-  if (a->count > b->count)
-    return 1;
-  else if (a->count < b->count)
-    return -1;
-  else
-    return value_compare_3way (&a->value, &b->value, vf->width);
-}
-
-/* Descending numeric compare of frequency;
-   secondary key on ascending numeric value. */
-static int
-compare_freq_numeric_d (const void *a_, const void *b_, const void *vf_ UNUSED)
-{
-  const struct freq *a = a_;
-  const struct freq *b = b_;
-
-  if (a->count > b->count)
-    return -1;
-  else if (a->count < b->count)
-    return 1;
-
-  if (a->value.f > b->value.f)
-    return 1;
-  else if (a->value.f < b->value.f)
-    return -1;
-  else
-    return 0;
-}
-
-/* Descending numeric compare of frequency;
-   secondary key on ascending string value. */
-static int
-compare_freq_alpha_d (const void *a_, const void *b_, const void *vf_)
-{
-  const struct freq *a = a_;
-  const struct freq *b = b_;
-  const struct var_freqs *vf = vf_;
-
-  if (a->count > b->count)
-    return -1;
-  else if (a->count < b->count)
-    return 1;
-  else
-    return value_compare_3way (&a->value, &b->value, vf->width);
-}
 \f
 /* Frequency table display. */
 
@@ -929,7 +860,7 @@ dump_freq_table (const struct var_freqs *vf, const struct variable *wv)
       if (label != NULL)
         tab_text (t, 0, r, TAB_LEFT, label);
 
-      tab_value (t, 1, r, TAB_NONE, &f->value, ft->dict, &vf->print);
+      tab_value (t, 1, r, TAB_NONE, &f->value, vf->var, NULL);
       tab_double (t, 2, r, TAB_NONE, f->count, wfmt);
       tab_double (t, 3, r, TAB_NONE, percent, NULL);
       tab_double (t, 4, r, TAB_NONE, valid_percent, NULL);
@@ -946,7 +877,7 @@ dump_freq_table (const struct var_freqs *vf, const struct variable *wv)
       if (label != NULL)
         tab_text (t, 0, r, TAB_LEFT, label);
 
-      tab_value (t, 1, r, TAB_NONE, &f->value, ft->dict, &vf->print);
+      tab_value (t, 1, r, TAB_NONE, &f->value, vf->var, NULL);
       tab_double (t, 2, r, TAB_NONE, f->count, wfmt);
       tab_double (t, 3, r, TAB_NONE,
                     f->count / ft->total_cases * 100.0, NULL);
@@ -969,106 +900,73 @@ dump_freq_table (const struct var_freqs *vf, const struct variable *wv)
 \f
 /* Statistical display. */
 
-/* Calculates all the pertinent statistics for variable V, putting them in
-   array D[]. */
+static double
+calc_percentile (double p, double valid_cases, double x1, double x2)
+{
+  double s, dummy;
+
+  s = (settings_get_algorithm () != COMPATIBLE
+       ? modf ((valid_cases - 1) * p, &dummy)
+       : modf ((valid_cases + 1) * p - 1, &dummy));
+
+  return x1 + (x2 - x1) * s;
+}
+
+/* Calculates all of the percentiles for VF within FRQ. */
 static void
-calc_stats (const struct var_freqs *vf, double d[FRQ_N_STATS])
+calc_percentiles (const struct frq_proc *frq, const struct var_freqs *vf)
 {
   const struct freq_tab *ft = &vf->tab;
   double W = ft->valid_cases;
-  struct moments *m;
-  struct freq *f=0;
-  int most_often;
-  double X_mode;
-
+  const struct freq *f;
+  int percentile_idx;
   double rank;
-  int i = 0;
-  int idx;
-
-  /* Calculate percentiles. */
 
   assert (ft->n_valid > 0);
 
-  for (i = 0; i < n_percentiles; i++)
-    {
-      percentiles[i].flag = 0;
-      percentiles[i].flag2 = 0;
-    }
-
   rank = 0;
-  for (idx = 0; idx < ft->n_valid; ++idx)
+  percentile_idx = 0;
+  for (f = ft->valid; f < ft->missing; f++)
     {
-      static double prev_value = SYSMIS;
-      f = &ft->valid[idx];
-      rank += f->count ;
-      for (i = 0; i < n_percentiles; i++)
+      rank += f->count;
+      for (; percentile_idx < frq->n_percentiles; percentile_idx++)
         {
-         double tp;
-         if ( percentiles[i].flag2  ) continue ;
+          struct percentile *pc = &frq->percentiles[percentile_idx];
+          double tp;
 
-         if ( settings_get_algorithm () != COMPATIBLE )
-           tp =
-             (ft->valid_cases - 1) *  percentiles[i].p;
-         else
-           tp =
-             (ft->valid_cases + 1) *  percentiles[i].p - 1;
+          tp = (settings_get_algorithm () == ENHANCED
+                ? (W - 1) * pc->p
+                : (W + 1) * pc->p - 1);
 
-         if ( percentiles[i].flag )
-           {
-             percentiles[i].x2 = f->value.f;
-             percentiles[i].x1 = prev_value;
-             percentiles[i].flag2 = 1;
-             continue;
-           }
+          if (rank <= tp)
+            break;
 
-          if (rank >  tp )
-         {
-           if ( f->count > 1 && rank - (f->count - 1) > tp )
-             {
-               percentiles[i].x2 = percentiles[i].x1 = f->value.f;
-               percentiles[i].flag2 = 1;
-             }
-           else
-             {
-               percentiles[i].flag=1;
-             }
-
-           continue;
-         }
+          if (tp + 1 < rank || f + 1 >= ft->missing)
+            pc->value = f->value.f;
+          else
+            pc->value = calc_percentile (pc->p, W, f->value.f, f[1].value.f);
         }
-      prev_value = f->value.f;
     }
-
-  for (i = 0; i < n_percentiles; i++)
+  for (; percentile_idx < frq->n_percentiles; percentile_idx++)
     {
-      /* Catches the case when p == 100% */
-      if ( ! percentiles[i].flag2 )
-       percentiles[i].x1 = percentiles[i].x2 = f->value.f;
-
-      /*
-      printf("percentile %d (p==%.2f); X1 = %g; X2 = %g\n",
-            i,percentiles[i].p,percentiles[i].x1,percentiles[i].x2);
-      */
+      struct percentile *pc = &frq->percentiles[percentile_idx];
+      pc->value = ft->valid[ft->n_valid - 1].value.f;
     }
+}
 
-  for (i = 0; i < n_percentiles; i++)
-    {
-      double s;
-
-      double dummy;
-      if ( settings_get_algorithm () != COMPATIBLE )
-       {
-         s = modf((ft->valid_cases - 1) * percentiles[i].p , &dummy);
-       }
-      else
-       {
-         s = modf((ft->valid_cases + 1) * percentiles[i].p -1, &dummy);
-       }
-
-      percentiles[i].value = percentiles[i].x1 +
-       ( percentiles[i].x2 - percentiles[i].x1) * s ;
-    }
+/* Calculates all the pertinent statistics for VF, putting them in array
+   D[]. */
+static void
+calc_stats (const struct var_freqs *vf, double d[FRQ_N_STATS])
+{
+  const struct freq_tab *ft = &vf->tab;
+  double W = ft->valid_cases;
+  const struct freq *f;
+  struct moments *m;
+  int most_often;
+  double X_mode;
 
+  assert (ft->n_valid > 0);
 
   /* Calculate the mode. */
   most_often = -1;
@@ -1112,7 +1010,8 @@ calc_stats (const struct var_freqs *vf, double d[FRQ_N_STATS])
 
 /* Displays a table of all the statistics requested for variable V. */
 static void
-dump_statistics (const struct var_freqs *vf, const struct variable *wv)
+dump_statistics (const struct frq_proc *frq, const struct var_freqs *vf,
+                 const struct variable *wv)
 {
   const struct fmt_spec *wfmt = wv ? var_get_print_format (wv) : &F_8_0;
   const struct freq_tab *ft = &vf->tab;
@@ -1130,8 +1029,9 @@ dump_statistics (const struct var_freqs *vf, const struct variable *wv)
       return;
     }
   calc_stats (vf, stat_value);
+  calc_percentiles (frq, vf);
 
-  t = tab_create (3, n_stats + n_show_percentiles + 2);
+  t = tab_create (3, frq->n_stats + frq->n_show_percentiles + 2);
 
   tab_box (t, TAL_1, TAL_1, -1, -1 , 0 , 0 , 2, tab_nr(t) - 1) ;
 
@@ -1142,7 +1042,7 @@ dump_statistics (const struct var_freqs *vf, const struct variable *wv)
   r=2; /* N missing and N valid are always dumped */
 
   for (i = 0; i < FRQ_N_STATS; i++)
-    if (stats & BIT_INDEX (i))
+    if (frq->stats & BIT_INDEX (i))
       {
        tab_text (t, 0, r, TAB_LEFT | TAT_TITLE,
                      gettext (st_name[i].s10));
@@ -1157,9 +1057,11 @@ dump_statistics (const struct var_freqs *vf, const struct variable *wv)
   tab_double (t, 2, 0, TAB_NONE, ft->valid_cases, wfmt);
   tab_double (t, 2, 1, TAB_NONE, ft->total_cases - ft->valid_cases, wfmt);
 
-  for (i = 0; i < n_percentiles; i++, r++)
+  for (i = 0; i < frq->n_percentiles; i++)
     {
-      if (!percentiles[i].show)
+      struct percentile *pc = &frq->percentiles[i];
+
+      if (!pc->show)
         continue;
 
       if ( i == 0 )
@@ -1167,12 +1069,13 @@ dump_statistics (const struct var_freqs *vf, const struct variable *wv)
          tab_text (t, 0, r, TAB_LEFT | TAT_TITLE, _("Percentiles"));
        }
 
-      if (percentiles[i].p == 0.5)
+      if (pc->p == 0.5)
         tab_text (t, 1, r, TAB_LEFT, _("50 (Median)"));
       else
-        tab_fixed (t, 1, r, TAB_LEFT, percentiles[i].p * 100, 3, 0);
-      tab_double (t, 2, r, TAB_NONE, percentiles[i].value,
-                 var_get_print_format (vf->var));
+        tab_fixed (t, 1, r, TAB_LEFT, pc->p * 100, 3, 0);
+      tab_double (t, 2, r, TAB_NONE, pc->value,
+                  var_get_print_format (vf->var));
+      r++;
     }
 
   tab_title (t, "%s", var_to_string (vf->var));
@@ -1181,18 +1084,20 @@ dump_statistics (const struct var_freqs *vf, const struct variable *wv)
 }
 
 static double
-calculate_iqr (void)
+calculate_iqr (const struct frq_proc *frq)
 {
   double q1 = SYSMIS;
   double q3 = SYSMIS;
   int i;
 
-  for (i = 0; i < n_percentiles; i++)
+  for (i = 0; i < frq->n_percentiles; i++)
     {
-      if (fabs (0.25 - percentiles[i].p) < DBL_EPSILON)
-        q1 = percentiles[i].value;
-      else if (fabs (0.75 - percentiles[i].p) < DBL_EPSILON)
-        q3 = percentiles[i].value;
+      struct percentile *pc = &frq->percentiles[i];
+
+      if (fabs (0.25 - pc->p) < DBL_EPSILON)
+        q1 = pc->value;
+      else if (fabs (0.75 - pc->p) < DBL_EPSILON)
+        q3 = pc->value;
     }
 
   return q1 == SYSMIS || q3 == SYSMIS ? SYSMIS : q3 - q1;
@@ -1216,14 +1121,14 @@ chart_includes_value (const struct frq_chart *chart,
 
 /* Create a gsl_histogram from a freq_tab */
 struct histogram *
-freq_tab_to_hist (const struct freq_tab *ft, const struct variable *var)
+freq_tab_to_hist (const struct frq_proc *frq, const struct freq_tab *ft,
+                  const struct variable *var)
 {
   double x_min, x_max, valid_freq;
   int i;
-
+  double bin_width;
   struct histogram *histogram;
   double iqr;
-  int bins;
 
   /* Find out the extremes of the x value, within the range to be included in
      the histogram, and sum the total frequency of those values. */
@@ -1232,45 +1137,39 @@ freq_tab_to_hist (const struct freq_tab *ft, const struct variable *var)
   valid_freq = 0;
   for (i = 0; i < ft->n_valid; i++)
     {
-      const struct freq *frq = &ft->valid[i];
-      if (chart_includes_value (&hist, var, &frq->value))
+      const struct freq *f = &ft->valid[i];
+      if (chart_includes_value (frq->hist, var, &f->value))
         {
-          x_min = MIN (x_min, frq->value.f);
-          x_max = MAX (x_max, frq->value.f);
-          valid_freq += frq->count;
+          x_min = MIN (x_min, f->value.f);
+          x_max = MAX (x_max, f->value.f);
+          valid_freq += f->count;
         }
     }
 
   /* Freedman-Diaconis' choice of bin width. */
-  iqr = calculate_iqr ();
-  if (iqr != SYSMIS)
-    {
-      double bin_width = 2 * iqr / pow (valid_freq, 1.0 / 3.0);
-      bins = (x_max - x_min) / bin_width;
-      if (bins < 5)
-        bins = 5;
-      else if (bins > 400)
-        bins = 400;
-    }
-  else
-    bins = 5;
+  iqr = calculate_iqr (frq);
+  bin_width = 2 * iqr / pow (valid_freq, 1.0 / 3.0);
+
+  histogram = histogram_create (bin_width, x_min, x_max);
+
+  if ( histogram == NULL)
+    return NULL;
 
-  histogram = histogram_create (bins, x_min, x_max);
   for (i = 0; i < ft->n_valid; i++)
     {
-      const struct freq *frq = &ft->valid[i];
-      if (chart_includes_value (&hist, var, &frq->value))
-        histogram_add (histogram, frq->value.f, frq->count);
+      const struct freq *f = &ft->valid[i];
+      if (chart_includes_value (frq->hist, var, &f->value))
+        histogram_add (histogram, f->value.f, f->count);
     }
 
   return histogram;
 }
 
 static int
-add_slice (const struct freq *freq, const struct variable *var,
-           struct slice *slice)
+add_slice (const struct frq_chart *pie, const struct freq *freq,
+           const struct variable *var, struct slice *slice)
 {
-  if (chart_includes_value (&pie, var, &freq->value))
+  if (chart_includes_value (pie, var, &freq->value))
     {
       ds_init_empty (&slice->label);
       var_append_value_name (var, &freq->value, &slice->label);
@@ -1286,7 +1185,8 @@ add_slice (const struct freq *freq, const struct variable *var,
    The caller is responsible for freeing slices
 */
 static struct slice *
-freq_tab_to_slice_array(const struct freq_tab *frq_tab,
+freq_tab_to_slice_array(const struct frq_chart *pie,
+                        const struct freq_tab *frq_tab,
                        const struct variable *var,
                        int *n_slicesp)
 {
@@ -1298,9 +1198,9 @@ freq_tab_to_slice_array(const struct freq_tab *frq_tab,
   n_slices = 0;
 
   for (i = 0; i < frq_tab->n_valid; i++)
-    n_slices += add_slice (&frq_tab->valid[i], var, &slices[n_slices]);
+    n_slices += add_slice (pie, &frq_tab->valid[i], var, &slices[n_slices]);
   for (i = 0; i < frq_tab->n_missing; i++)
-    n_slices += add_slice (&frq_tab->missing[i], var, &slices[n_slices]);
+    n_slices += add_slice (pie, &frq_tab->missing[i], var, &slices[n_slices]);
 
   *n_slicesp = n_slices;
   return slices;
@@ -1310,12 +1210,13 @@ freq_tab_to_slice_array(const struct freq_tab *frq_tab,
 
 
 static void
-do_piechart(const struct variable *var, const struct freq_tab *frq_tab)
+do_piechart(const struct frq_chart *pie, const struct variable *var,
+            const struct freq_tab *frq_tab)
 {
   struct slice *slices;
   int n_slices, i;
 
-  slices = freq_tab_to_slice_array (frq_tab, var, &n_slices);
+  slices = freq_tab_to_slice_array (pie, frq_tab, var, &n_slices);
 
   if (n_slices < 2)
     msg (SW, _("Omitting pie chart for %s, which has only %d unique values."),