Bug 52125 - StatCalculator.addAll(StatCalculator<T> calc) joins incorrect if there are more samples with the same response time in one of the TreeMap
Summary: StatCalculator.addAll(StatCalculator<T> calc) joins incorrect if there are mo...
Status: RESOLVED FIXED
Alias: None
Product: JMeter - Now in Github
Classification: Unclassified
Component: Main (show other bugs)
Version: 2.3.4
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks: 52339
  Show dependency tree
 
Reported: 2011-11-02 15:51 UTC by alena
Modified: 2011-12-20 20:51 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description alena 2011-11-02 15:51:37 UTC
When joining two StatCalucators (where a key in the TreeMap has value > 1) I got incorrect values of the final dataset (count, sum, deviation ..) 

Problematic functions are addAll(StatCalculator<T> calc) and addValue(T val, int sampleCount)

See example showing the behavior and proposed fix of the functions.

==========================================================================
EXAMPLE
Let’s have two StatCalculator sc1 and sc2. 
sc1 has values: 1, 2, 3
sc2 has values: 2, 2, 2

STAT.VALUES OF sc1 
count: 3
sum: 6.0
deviation: 0.8164965809277263
TreeMap: 
• key: 1 value: 1
• key: 2 value: 1
• key: 3 value: 1

STAT.VALUES OF sc2 
count: 3
sum: 6.0
deviation: 0.0
TreeMap: 
• value: 2 count: 3

STAT.VALUES OF sc1+sc2  (sc1.addAll(sc2))
count: 4   WRONG (should be 6)
sum: 8.0   WRONG (should be 12)
deviation: 0.707106711865476 WRONG (should be 0.5773502691896255)
TreeMap: 
• key: 1 value: 1
• key: 2 value: 2 WRONG(samples with response=2ms occurs 4times!)                  
• key: 3 value: 1

==========================================================================
ORIGINAL FUNCTIONS:

public void addAll(StatCalculator<T> calc) {
        for (T val : calc.valuesMap.keySet()) {
            addValue(val);
        }
    }

public void addValue(T val, int sampleCount) {
        count += sampleCount;
        double currentVal = val.doubleValue();
        sum += currentVal;
        T actualValue = val;
        if (sampleCount > 1){
            // For n values in an aggregate sample the average value = (val/n)
            // So need to add n * (val/n) * (val/n) = val * val / n
            sumOfSquares += currentVal * currentVal / sampleCount;
            actualValue = divide(val, sampleCount);
        } else {
            sumOfSquares += currentVal * currentVal;
            actualValue = val;
        }
        updateValueCount(actualValue, sampleCount);
        mean = sum / count;
        deviation = Math.sqrt((sumOfSquares / count) - (mean * mean));
        if (actualValue.compareTo(max) > 0){
            max=actualValue;
        }
        if (actualValue.compareTo(min) < 0){
            min=actualValue;
        }
    }
====================================================================
FIXED FUNCTIONS:

public void addAll(StatCalculator<T> calc) {
        for (T val : calc.valuesMap.keySet()) {
        	addValue(val , calc.valuesMap.get(val).intValue());
        }
    }


public void addValue(T val, int sampleCount) {
        count += sampleCount;
        double currentVal = val.doubleValue();   
        sum += currentVal * sampleCount; 
        // For n same values in sum of square is equal to n*val^2
        sumOfSquares += currentVal * currentVal * sampleCount; 
        updateValueCount(val, sampleCount);       
        mean = sum / count;
        deviation = Math.sqrt((sumOfSquares / count) - (mean * mean));
        if (val.compareTo(max) > 0){
            max=val;
        }
        if (val.compareTo(min) < 0){
            min=val;
        }
    }
Comment 1 Sebb 2011-11-04 00:59:30 UTC
Thanks! Good examples and fix.
Ideally patches should be provided as unified diffs as these are easier to review and apply, but in this case it was easy to see the changes.

Applied with one minor change: used entrySet for the iterator as now need the value as well as the key.

URL: http://svn.apache.org/viewvc?rev=1197376&view=rev
Log:
Bug 52125 - StatCalculator.addAll(StatCalculator calc) joins incorrect if there are more samples with the same response time in one of the TreeMap

Modified:
   jmeter/trunk/src/jorphan/org/apache/jorphan/math/StatCalculator.java
   jmeter/trunk/test/src/org/apache/jorphan/math/TestStatCalculator.java
   jmeter/trunk/xdocs/changes.xml
Comment 2 Sebb 2011-12-17 19:20:46 UTC
The fix broke processing of StatisticalSampleResults.

The problem is that the StatisticalSampleResults class accumulates the elapsed time for each sample, whereas the StatCalculator classes count samples with the same values. In the first case, there is no need to multiply by the number of samples, but in the second the value needs to be adjusted before use.

Also need to add some tests for the StatisticalSampleResults class.
Comment 3 Sebb 2011-12-20 20:51:46 UTC
URL: http://svn.apache.org/viewvc?rev=1221486&view=rev
Log:
Bug 52339 - JMeter Statistical mode in distributed testing shows wrong response time<

Modified:
   jmeter/trunk/src/jorphan/org/apache/jorphan/math/StatCalculator.java
   jmeter/trunk/src/jorphan/org/apache/jorphan/math/StatCalculatorInteger.java
   jmeter/trunk/src/jorphan/org/apache/jorphan/math/StatCalculatorLong.java
   jmeter/trunk/test/src/org/apache/jorphan/math/TestStatCalculator.java
   jmeter/trunk/xdocs/changes.xml
Comment 4 The ASF infrastructure team 2022-09-24 20:37:47 UTC
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/2632