[ltt-dev] [PATCH 1/2] Use appropriate factor-pair when reducing

[Benjamin Poirier]

Correct a bug in one of the factor reduction functions.

To quote the README:
	Event analysis yields time correction factors between trace pairs. For
	groups of more than two traces, an extra step is needed to identify a
	reference trace and calculate correction factors for each trace relative
	to this reference.

After arranging the traces in a tree, getFactors() finds correction factors to
convert a trace's time to its reference's time. It does this by recursively
combining the factors between the trace and its parent with the factors
between the parent and the reference. Example:

A
 \_ B
 \_ C
     \_ D

With the following tree, calling getFactors() on D should combine the factors
between C and D with the result of calling getFactors() on C. (Calling
getFactors() on the reference, A, yields "identity" factors.)

The current implementation of getFactors() has a bug. When combining, it uses
the factors between a node and the reference instead of the factors between a
node and its parent. This is a problem on nodes with detph >= 2 and triggers
an assert(). This patch corrects the bug.

Signed-off-by: Benjamin Poirier <benjamin.poirier at polymtl.ca>
Reported-by: Masoume Jabbarifar <masoume.jabbarifar at polymtl.ca>
---

The fix suggested earlier by Masoume achieves the same end goal. This one
however doesn't add an extra conditionnal block. (A little simpler, perharps
because that's how the original author had meant to implement it in the first
place ;)

 lttv/lttv/sync/factor_reduction_accuracy.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/lttv/lttv/sync/factor_reduction_accuracy.c b/lttv/lttv/sync/factor_reduction_accuracy.c
index a63dae7..885c52c 100644
--- a/lttv/lttv/sync/factor_reduction_accuracy.c
+++ b/lttv/lttv/sync/factor_reduction_accuracy.c
@@ -425,30 +425,31 @@ static void getFactors(AllFactors* const allFactors, unsigned int** const
 	else
 	{
 		Factors previousVertexFactors;
+		unsigned int previousVertex= predecessors[reference][traceNum];
 
-		getFactors(allFactors, predecessors, references,
-			predecessors[reference][traceNum], &previousVertexFactors);
+		getFactors(allFactors, predecessors, references, previousVertex,
+			&previousVertexFactors);
 
 		/* Convert the time from traceNum to reference;
 		 * pairFactors[row][col] converts the time from col to row, invert the
 		 * factors as necessary */
 
-		if (pairFactors[reference][traceNum].approx != NULL)
+		if (pairFactors[previousVertex][traceNum].approx != NULL)
 		{
 			factors->offset= previousVertexFactors.drift *
-				pairFactors[reference][traceNum].approx->offset +
+				pairFactors[previousVertex][traceNum].approx->offset +
 				previousVertexFactors.offset;
 			factors->drift= previousVertexFactors.drift *
-				pairFactors[reference][traceNum].approx->drift;
+				pairFactors[previousVertex][traceNum].approx->drift;
 		}
-		else if (pairFactors[traceNum][reference].approx != NULL)
+		else if (pairFactors[traceNum][previousVertex].approx != NULL)
 		{
 			factors->offset= previousVertexFactors.drift * (-1. *
-				pairFactors[traceNum][reference].approx->offset /
-				pairFactors[traceNum][reference].approx->drift) +
+				pairFactors[traceNum][previousVertex].approx->offset /
+				pairFactors[traceNum][previousVertex].approx->drift) +
 				previousVertexFactors.offset;
 			factors->drift= previousVertexFactors.drift * (1. /
-				pairFactors[traceNum][reference].approx->drift);
+				pairFactors[traceNum][previousVertex].approx->drift);
 		}
 		else
 		{
-- 
1.7.1