f9d03e79d5565c0d9a28d57c0c67986a085a40c4
kent
  Tue Dec 14 21:09:37 2021 -0800
Fixing a three little bugs.  Adding some #defines to avoid magic numbers. Fixing differences in sorting between expression values and labels.

diff --git src/hg/lib/facetedTable.c src/hg/lib/facetedTable.c
index 0f053d4..84978b7 100644
--- src/hg/lib/facetedTable.c
+++ src/hg/lib/facetedTable.c
@@ -3,30 +3,34 @@
 
 #include "common.h"
 #include "hash.h"
 #include "hdb.h"
 #include "web.h"
 #include "trashDir.h"
 #include "hCommon.h"
 #include "htmlColor.h"
 #include "hgColors.h"
 #include "fieldedTable.h"
 #include "tablesTables.h"
 #include "facetField.h"
 #include "hgConfig.h"
 #include "facetedTable.h"
 
+/* These two may someday need to be non-constant */
+#define ftClusterOutIx 0    /* Output for synthesized cluster name is always in zero column */
+#define ftKeySep '/'	    /* Componenets of composite keys are separated with they */
+
 static struct facetedTable *facetedTableNew(char *name, char *varPrefix, char *facets)
 /* Return new, mostly empty faceted table */
 {
 struct facetedTable *facTab;
 AllocVar(facTab);
 facTab->name = cloneString(name);
 facTab->varPrefix = cloneString(varPrefix);
 facTab->facets = cloneString(facets);
 return facTab;
 }
 
 struct facetedTable *facetedTableFromTable(struct fieldedTable *table, 
     char *varPrefix, char *facets)
 /* Construct a facetedTable around a fieldedTable */
 {
@@ -134,37 +138,39 @@
 	char selListVar[256];
 	safef(selListVar, sizeof(selListVar), "%s_facet_selList", facTab->varPrefix);
 	char *selectedFacetValues=cartUsualString(cart, selListVar, "");
 	struct facetField *selList = deLinearizeFacetValString(selectedFacetValues);
 	selectedListFacetValUpdate(&selList, selFieldName, selFieldVal, selOp);
 	char *newSelectedFacetValues = linearizeFacetVals(selList);
 	cartSetString(cart, selListVar, newSelectedFacetValues);
 	facetedTableRemoveOpVars(facTab, cart);
 	}
     return TRUE;
     }
 else
     return FALSE;
 }
 
+
 struct mergeGroup
 /* Adds up series of counts and values with values weighted by count*/
     {
     struct mergeGroup *next;
-    char *key;	        /* Key - just / separated list of component keys */
-    char **aRow;	/* First row we encounter in input table*/
+    char *key;		     /* Unique key for this group */
+    struct fieldedRow *aRow; /* First row we encounter in input table */
     struct slRef *rowRefList;  /* List of refs to all rows (struct fieldedRow *) in merge */
+    // struct mergeColHelper *col;  /* Associated column if any */
     };
 
 struct mergeColHelper
 /* Helper structure for merging - one per column whether merged or not */
     {
     struct mergeColHelper *next;
     char *name;	/* Name of column */
     int ix;	/* Column index in table */
     boolean isMerged;	/* If true it's a merging column */
     boolean isFacet;	/* If true it's a faceting column */
     boolean sameAfterMerger;  /* If true then all values after merge would be same */
     boolean isCount;	/* Is count field */
     boolean isVal;	/* Is val field */
     boolean isKey;	/* Is it the key field */
     boolean isColor;	/* Is it the color field? */
@@ -199,54 +205,54 @@
     helper->name = field;
     helper->ix = i;
     struct facetField *ff = hashFindVal(ffHash, field);
     if (ff != NULL)
 	helper->isMerged = ff->isMerged;
     helper->isFacet = (hashLookup(facetHash, field) != NULL);
     helper->isVal = sameString(field, "val");
     if (helper->isVal)
         *retValCol = helper;
     helper->isCount = sameString(field, "count");
     if (helper->isCount)
         *retCountCol = helper;
     helper->isColor = sameString(field, "color");
     if (helper->isColor)
         *retColorCol = helper;
-    helper->isKey = (i == 0);
+    helper->isKey = (i == ftClusterOutIx);
     slAddHead(&list, helper);
     }
 slReverse(&list);
 
 /* Clean up and go home */
 hashFree(&ffHash);
 hashFree(&facetHash);
 slNameFreeList(&facetNameList);
 return list;
 }
 
 static char *calcMergedKey(struct mergeColHelper *colList, char **row, 
     struct dyString *key)
 /* Calculate merging key for this row. Puts result in key, whose contents will be overwritten */
 {
 dyStringClear(key);
 struct mergeColHelper *col;
 for (col = colList; col != NULL; col = col->next)
     {
     if (col->isFacet && !col->isMerged)
 	{
 	if (key->stringSize != 0)
-	    dyStringAppendC(key, '/');
+	    dyStringAppendC(key, ftKeySep);
 	dyStringAppend(key, row[col->ix]);
 	}
     }
 return key->string;
 }
 
 long summedCountAt(int countIx, struct mergeGroup *merge)
 /* Return sum of column across merge group */
 {
 long long total = 0;
 struct slRef *ref;
 for (ref = merge->rowRefList; ref != NULL; ref = ref->next)
     {
     struct fieldedRow *fr = ref->val;
     char **row = fr->row;
@@ -262,49 +268,49 @@
 {
 int maxVal = 240;
 int threshold = 100;
 if (count == 0)
     return maxVal;  // Which makes us nearly white and invisible
 double fullColor = colSum/count;
 int underThreshold = threshold - count;
 if (underThreshold < 0)
     return round(fullColor);
 
 /* We are under threshold.  Blend us with white (aka maxVal) */
 double ratioUnderThreshold = (double)underThreshold/(double)threshold;
 return (1 - ratioUnderThreshold)*fullColor + ratioUnderThreshold*maxVal;
 }
 
-long long weightedAveValAt(int countIx, int valIx, struct mergeGroup *merge)
+double weightedAveValAt(int countIx, int valIx, struct mergeGroup *merge)
 /* Return wieghted average value */
 {
 long totalCount = 0;
 double weightedSum = 0;
 struct slRef *ref;
 for (ref = merge->rowRefList; ref != NULL; ref = ref->next)
     {
     struct fieldedRow *fr = ref->val;
     char **row = fr->row;
     int count = sqlUnsigned(row[countIx]);
     double val = sqlDouble(row[valIx]);
     totalCount += count;
     weightedSum += val*count;
     }
 return weightedSum/totalCount;
 }
 
-long long aveValAt(int valIx, struct mergeGroup *merge)
+double aveValAt(int valIx, struct mergeGroup *merge)
 /* Return average value */
 {
 int count = 0;
 double weightedSum = 0;
 struct slRef *ref;
 for (ref = merge->rowRefList; ref != NULL; ref = ref->next)
     {
     struct fieldedRow *fr = ref->val;
     char **row = fr->row;
     double val = sqlDouble(row[valIx]);
     count += 1;
     weightedSum += val;
     }
 return weightedSum/count;
 }
@@ -342,129 +348,160 @@
     struct mergeColHelper *colList)
 /* Return list of mergeGroups corresponding to colList */
 {
 struct mergeGroup *mergeList = NULL;
 struct hash *mergeHash = hashNew(0);
 struct dyString *keyBuf = dyStringNew(0);
 struct fieldedRow *fr;
 for (fr = tableIn->rowList; fr != NULL; fr = fr->next)
     {
     char *key = calcMergedKey(colList, fr->row, keyBuf);
     struct mergeGroup *merge = hashFindVal(mergeHash, key);
     if (merge == NULL)
         {
 	AllocVar(merge);
 	merge->key = cloneString(key);
-	merge->aRow = fr->row;
-	slAddHead(&mergeList, merge);
+	merge->aRow = fr;
 	hashAdd(mergeHash, key, merge);
+	slAddHead(&mergeList, merge);
 	}
     refAdd(&merge->rowRefList, fr);
     }
+
+/* Do list reversals, damn cheap singly-linked lists always needing this! */
+struct mergeGroup *merge;
+for (merge = mergeList; merge != NULL; merge = merge->next)
+    slReverse(&merge->rowRefList);
 slReverse(&mergeList);
 
 /* Clean up and go home. */
 dyStringFree(&keyBuf);
 hashFree(&mergeHash);
 return mergeList;
 }
 
-
-static char **annotateColListAndMakeNewFields(struct mergeColHelper *colList, 
-    struct mergeGroup *mergeList, int *retCount)
+#ifdef DEBUG
+static struct hash *hashColList(struct mergeColHelper *colList)
+/* Make hash of colList keyed by name */
 {
-/* Make a trip through merged table noting whether a each column merges cleanly */
+   { // ugly debug
+   struct dyString *dy = dyStringNew(0);
     struct mergeColHelper *col;
     for (col = colList; col != NULL; col = col->next)
+	dyStringPrintf(dy, "%s,", col->name);
+    uglyAbort("hashColList on %s", dy->string);
+   }
+struct hash *hash = hashNew(0);
+struct mergeColHelper *col;
+for (col = colList; col != NULL; col = col->next)    hashAdd(hash, col->name, col);
+    hashAdd(hash, col->name, col);
+return hash;
+}
+#endif /* DEBUG */
+
+void annotateColListAndMakeNewFields(struct mergeColHelper *colList, 
+    struct mergeGroup *mergeList, int *retCount, struct mergeColHelper ***retNewFields)
+/* Update fields in colList. */
 {
 struct mergeGroup *merge;
+struct mergeColHelper *col;
+
+/* Make a trip through merged table noting whether a each column merges cleanly */
+for (col = colList; col != NULL; col = col->next)
+    {
     int colIx = col->ix;
     boolean mergeOk = TRUE;
     for (merge = mergeList; merge != NULL && mergeOk; merge = merge->next)
         {
 	struct slRef *ref = merge->rowRefList;
 	struct fieldedRow *fr = ref->val;
 	char **row = fr->row;
 	char *first = row[colIx];
 	for (ref = ref->next; ref != NULL; ref = ref->next)
 	    {
 	    fr = ref->val;
 	    row = fr->row;
 	    if (!sameString(first, row[colIx]))
 	        {
 		mergeOk = FALSE;
 		break;
 		}
 	    }
 	}
     col->sameAfterMerger = mergeOk;
     }
 
 /* Figure out list of fields for new table 
  *     We'll keep the first field but repurpose it as new index.
  *     We'll copy over the non-merged facet fields
  *     We'll copy over other fields that have the same value for each row in the merge
  *     We'll compute new count and val fields where count is sum and val is weighted average 
  *     If there's a color field we'll merge it by weighted average as well */
 
-char **newFields;
+struct mergeColHelper **newFields;
 AllocArray(newFields, slCount(colList)); // Big enough
 int newFieldCount = 0;
 for (col = colList; col != NULL; col = col->next)
     {
-    char *name = col->name;
     if (col->sameAfterMerger || col->isKey || col->isCount || col->isVal || col->isColor)
         {
-	newFields[newFieldCount] = name;
+	newFields[newFieldCount] = col;
 	col->mergedIx = newFieldCount;
 	newFieldCount += 1;
 	}
     else
         col->mergedIx = -1;
     }
 *retCount = newFieldCount;
-return newFields;
+*retNewFields = newFields;
 }
 
 static struct facetField **recomputeFfArray(struct facetField **oldFfArray, 
     struct mergeColHelper *colList, int newFieldCount)
 /* Return freshly computed ffArray with some of bits of oldArray skipped */
 {
 /* Recompute ffArray */
 struct facetField **newFfArray;
 AllocArray(newFfArray, newFieldCount);
 struct mergeColHelper *col;
 for (col = colList; col != NULL; col = col->next)
     {
     if (col->mergedIx != -1)
         newFfArray[col->mergedIx] = oldFfArray[col->ix];
     }
 return newFfArray;
 }
 
 static struct fieldedTable *groupColumns(struct facetedTable *facTab, struct fieldedTable *tableIn,
     struct facetField *selList, struct facetField ***retFfArray)
 /* Create a new table that has somewhat fewer fields than the input table based on grouping
  * together columns based on isMerged values of selList.  Returns a new ffArray to go with new
  * table. */
 {
 struct mergeColHelper *countCol = NULL, *valCol = NULL, *colorCol = NULL;
 struct mergeColHelper *colList = makeColList(facTab, tableIn, selList, 
     &countCol, &valCol, &colorCol);
 struct mergeGroup *mergeList = findMergeGroups(tableIn, colList);
 int newFieldCount;
-char **newFields = annotateColListAndMakeNewFields(colList, mergeList, &newFieldCount);
+struct mergeColHelper **outCols;
+annotateColListAndMakeNewFields(colList, mergeList, &newFieldCount, &outCols);
+
+/* Convert array of columns to array of names */
+char *newFields[newFieldCount];
+int i;
+for (i=0; i<newFieldCount; ++i)
+    newFields[i] = outCols[i]->name;
 
 struct fieldedTable *tableOut = fieldedTableNew("merged", newFields, newFieldCount);
 char *newRow[newFieldCount];
 struct mergeGroup *merge;
 for (merge = mergeList; merge != NULL; merge = merge->next)
     {
     char countBuf[32], valBuf[32], colorBuf[8];
     struct mergeColHelper *col;
     for (col = colList; col != NULL; col = col->next)
         {
 	if (col->mergedIx != -1)
 	    {
 	    if (col->isKey)
 	        {
 		newRow[col->mergedIx] = merge->key;
@@ -482,31 +519,31 @@
 		    val = aveValAt(col->ix, merge);
 		else
 		    val = weightedAveValAt(countCol->ix, col->ix, merge);
 		safef(valBuf, sizeof(valBuf), "%g", val);
 		newRow[col->mergedIx] = valBuf;
 		}
 	    else if (col->isColor)
 	        {
 		int r,g,b;
 		weightedAveColorAt(countCol, col->ix, merge, &r, &g, &b);
 		safef(colorBuf, sizeof(colorBuf), "#%02X%02X%02X", r, g, b);
 		newRow[col->mergedIx] = colorBuf;
 		}
 	    else if (col->sameAfterMerger)
 	        {
-		newRow[col->mergedIx] = merge->aRow[col->ix];
+		newRow[col->mergedIx] = merge->aRow->row[col->ix];
 		}
 	    }
 	}
     fieldedTableAdd(tableOut, newRow, newFieldCount, tableOut->rowCount);
     }
 
 *retFfArray = recomputeFfArray(facTab->ffArray, colList, newFieldCount);
 return tableOut;
 }
 
 struct fieldedTable *facetedTableSelect(struct facetedTable *facTab, struct cart *cart,
     struct facetField ***retFfArray)
 /* Return table containing rows of table that have passed facet selection */
 {
 char *selList = facetedTableSelList(facTab, cart);
@@ -543,88 +580,96 @@
 	}
     ++ix;
     }
 slReverse(&retList);
 return retList;
 }
 
 static int facetedTableMergedOffsetCmp(const void *va, const void *vb)
 /* Compare to sort based on name start. */
 {
 const struct facetedTableMergedOffset *a = *((struct facetedTableMergedOffset **)va);
 const struct facetedTableMergedOffset *b = *((struct facetedTableMergedOffset **)vb);
 return cmpWordsWithEmbeddedNumbers(a->name, b->name);
 }
 
-
 struct facetedTableMergedOffset *facetedTableMakeMergedOffsets(struct facetedTable *facTab,
      struct cart *cart)
 /* Return a structure that will let us relatively rapidly merge together one row */
 {
 /* Figure out user selection of fields to select on and merge as ffSelList */
 char *selList = facetedTableSelList(facTab, cart);
 struct facetField *ffSelList  = deLinearizeFacetValString(selList);
 
 /* Get output column list and pointers to key columns */
 struct fieldedTable *tableIn = facTab->table;
 struct mergeColHelper *countCol = NULL, *valCol = NULL, *colorCol = NULL;
 struct mergeColHelper *colList = makeColList(facTab, tableIn, ffSelList, 
     &countCol, &valCol, &colorCol);
 struct mergeGroup *mergeList = findMergeGroups(tableIn, colList);
 int newFieldCount;
-annotateColListAndMakeNewFields(colList, mergeList, &newFieldCount);
+struct mergeColHelper **outCol;
+annotateColListAndMakeNewFields(colList, mergeList, &newFieldCount, &outCol);
 
 fieldedTableResetRowIds(tableIn, 0);	// Use the seldom-used ID column as indexes
 struct facetedTableMergedOffset *tmoList = NULL;
 struct mergeGroup *merge;
-struct dyString *keyBuf = dyStringNew(0);
-int outIx = 0;
+
 for (merge = mergeList; merge != NULL; merge = merge->next)
     {
     struct facetedTableMergedOffset *tmo;
     AllocVar(tmo);
     slAddHead(&tmoList, tmo);
-    tmo->outIx = outIx++;
-    dyStringClear(keyBuf);
     tmo->name = merge->key;
     int r,g,b;
     weightedAveColorAt(countCol, colorCol->ix, merge, &r, &g, &b);
     safef(tmo->color, sizeof(tmo->color), "#%02X%02X%02X", r, g, b);
 
     struct slRef *ref;
     for (ref = merge->rowRefList; ref != NULL; ref = ref->next)
         {
 	struct fieldedRow *fr = ref->val;
 	struct facetedTableCountOffset *tco;
 	AllocVar(tco);
 	tco->valIx = fr->id;
 	int count = tco->count = (countCol != NULL ? sqlUnsigned(fr->row[countCol->ix]) : 1);
 	tmo->totalCount += count;
 	slAddHead(&tmo->colList, tco);
 	}
+    slReverse(&tmo->colList);
     }
+
+/* Sort and attach ascending output indexes. */
 slSort(&tmoList, facetedTableMergedOffsetCmp);
+int outIx = 0;
+struct facetedTableMergedOffset *tmo;
+for (tmo = tmoList; tmo != NULL; tmo = tmo->next)
+    {
+    tmo->outIx = outIx++;
+    }
 return tmoList;
 }
 
-void facetedTableMergeVals(struct facetedTableMergedOffset *tmoList, float *inVals, float *outVals)
+void facetedTableMergeVals(struct facetedTableMergedOffset *tmoList, float *inVals, int inValCount,
+    float *outVals, int outValCount)
 /* Populate outVals array with columns of weighted averages derived from applying
  * tmoList to inVals array. */
 {
 struct facetedTableMergedOffset *tmo;
 for (tmo = tmoList; tmo != NULL; tmo = tmo->next)
     {
+    if (tmo->outIx < 0 || tmo->outIx >= outValCount) errAbort("facetTableMergeVals finds outIx out of range %d not in [0-%d)\n", tmo->outIx, outValCount);
     struct facetedTableCountOffset *colList = tmo->colList;
     if (colList->next == NULL)  // specail case of one
         {
 	outVals[tmo->outIx] = inVals[colList->valIx];
 	}
     else
         {
 	double scale = 1.0/tmo->totalCount;
 	double weightedSum = 0;
 	struct facetedTableCountOffset *col;
 	for (col = colList; col != NULL; col = col->next)
 	    {
 	    weightedSum += inVals[col->valIx] * (double)col->count;
 	    }
 	outVals[tmo->outIx] = weightedSum*scale;