I missed seeing this on reviewboard.  Can you provide an example of where
this was giving the wrong results?

I'm concerned that perhaps saying X/Y for vector values of X and Y is just
ambiguous, and changing the definition like this may make it match one
person's expectations in one place while also breaking someone else's
expectations somewhere else.

Or maybe I'm just confused by the comment... I see this fix is only in the
total() operation, but that's not mentioned in the description.

Steve

On Mon, Jun 4, 2012 at 10:24 PM, William Wang <[email protected]> wrote:

> changeset 4c0f7e5ae72a in /z/repo/gem5
> details: http://repo.gem5.org/gem5?cmd=changeset;node=4c0f7e5ae72a
> description:
>        stats: when applying an operation to two vectors sum the components
> first.
>
>        Previously writing X/Y in a formula would result in:
>        x[0]/y[0] + x[1]/y[1]
>        In reality you want:
>        (x[0] +x[1])/(y[0] + y[1])
>
> diffstat:
>
>  src/base/statistics.hh |  24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
>
> diffs (36 lines):
>
> diff -r 9cad1c26c3b3 -r 4c0f7e5ae72a src/base/statistics.hh
> --- a/src/base/statistics.hh    Tue Jun 05 01:23:11 2012 -0400
> +++ b/src/base/statistics.hh    Tue Jun 05 01:23:11 2012 -0400
> @@ -2289,9 +2289,31 @@
>     total() const
>     {
>         const VResult &vec = this->result();
> +        const VResult &lvec = l->result();
> +        const VResult &rvec = r->result();
>         Result total = 0.0;
> -        for (off_type i = 0; i < size(); i++)
> +        Result lsum = 0.0;
> +        Result rsum = 0.0;
> +        Op op;
> +
> +        assert(lvec.size() > 0 && rvec.size() > 0);
> +        assert(lvec.size() == rvec.size() ||
> +               lvec.size() == 1 || rvec.size() == 1);
> +
> +        /** If vectors are the same divide their sums (x0+x1)/(y0+y1) */
> +        if (lvec.size() == rvec.size() && lvec.size() > 1) {
> +            for (off_type i = 0; i < size(); ++i) {
> +                lsum += lvec[i];
> +                rsum += rvec[i];
> +            }
> +            return op(lsum, rsum);
> +        }
> +
> +        /** Otherwise divide each item by the divisor */
> +        for (off_type i = 0; i < size(); ++i) {
>             total += vec[i];
> +        }
> +
>         return total;
>     }
>
> _______________________________________________
> gem5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/gem5-dev
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to