From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 26F81A0C3F; Tue, 23 Mar 2021 18:24:36 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0DE99140F66; Tue, 23 Mar 2021 18:24:36 +0100 (CET) Received: from mail-oi1-f182.google.com (mail-oi1-f182.google.com [209.85.167.182]) by mails.dpdk.org (Postfix) with ESMTP id AB2884069E for ; Tue, 23 Mar 2021 18:24:34 +0100 (CET) Received: by mail-oi1-f182.google.com with SMTP id c16so8927541oib.3 for ; Tue, 23 Mar 2021 10:24:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=VdbObF+DBIZb9MQjKYEJBSFJ2qd8pkaSjVYh6KHYTaE=; b=CXBTvV9MqED1KIFa1ZRSu/NMMxqJXcp6X3opWPgkglvxt+GtQKeEsHtLDf8tm7cqEn NyRmnf1FNQd2OenIgDZHgBjOk4yxRwFBL4knFr1kb+2JaKrA53k5T4/0E7uVxVhQoF2z qfPUJwU4/c9Aq0q3sADfB+jZxZQ+tgzyr2AI4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VdbObF+DBIZb9MQjKYEJBSFJ2qd8pkaSjVYh6KHYTaE=; b=d9qpEe+WFDytUs9PNhIjmOy04nV2lWUKueY7bmDp4VSmoh/zXFEQDi7HEvs2bnZEmg gst92TtrSDIrc+LF7QjlVJQOOyotU1VFXT9iY7OphEQv/8JjA6oFTx4TJorlMMdt/7Ux tPKXk8A9aX++qG7+3dMlpj47xEYsNrIg/qpt6NO3GXLzTzIstwc3GgS2Do5vN9niErua Pw7juq0IYs8Fx9ypaatBlYbZsniLH6KW6GbhhJOhD/7AzXdZjrq8ynzh01tQkPprAxiz xWEO3WMYu2N+8aH3ZWritkjFjDgbiIotYIVwRbHRZyVmpDMIELXb/URCbKrC9/lm8odp T9Ew== X-Gm-Message-State: AOAM533kMfAaSdq6/8SkkLp59Ov+pbdPAAw97OZFfb3AnAl59wY+9oq9 F9bjKVMI9qWXq/4NlYzNkVGEbQkFYz3hlkYaBDp+Og== X-Google-Smtp-Source: ABdhPJzGmp2iSxjZ4Gc5WinusitixforAZlEh0BtLfXRWI6i8rx7/BQFutkn/7/6C1kcBdYQmOS2szQFqi51r+wo1Ck= X-Received: by 2002:aca:530e:: with SMTP id h14mr4110502oib.79.1616520273920; Tue, 23 Mar 2021 10:24:33 -0700 (PDT) MIME-Version: 1.0 References: <20210311104800.243752-1-lijuan.tu@intel.com> <0e4279f9e86442ed81df0416e9e19ffd@intel.com> In-Reply-To: From: Owen Hilyard Date: Tue, 23 Mar 2021 13:23:55 -0400 Message-ID: To: "Tu, Lijuan" Cc: David Liu , Ali Alnubani , "dts@dpdk.org" , "dpdklab@iol.unh.edu" Content-Type: multipart/alternative; boundary="000000000000b31e3205be37744f" Subject: Re: [dts] [dpdklab] Re: [PATCH] tests/nic_single_core_perf: modify accepted_tolerance to percentage X-BeenThere: dts@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: test suite reviews and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dts-bounces@dpdk.org Sender: "dts" --000000000000b31e3205be37744f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello Lijuan, Would it be possible to have the delta be a float in the json representation? For example, a 20% delta would be encoded as "delta": .2 in json. Currently, most of the community lab's infrastructure expects to make pass/fail determinations on a numeric type, so having the delta as a float would reduce the required downtime to change everything over to percentages since we shouldn't need to patch all of our services to expect a string. Thanks, Owen Hilyard On Tue, Mar 23, 2021 at 10:53 AM David Liu wrote: > Thank you for Ali and Lijuan. > > We had tested the patch on the Arm machines setup. Other than the value > issues, the test is running as expected. > > On Tue, Mar 23, 2021 at 4:48 AM Tu, Lijuan wrote: > >> >> >> > -----Original Message----- >> > From: Ali Alnubani >> > Sent: 2021=E5=B9=B43=E6=9C=8823=E6=97=A5 16:44 >> > To: Tu, Lijuan >> > Cc: dts@dpdk.org; dpdklab@iol.unh.edu >> > Subject: RE: [dts] [PATCH] tests/nic_single_core_perf: modify >> > accepted_tolerance to percentage >> > >> > > -----Original Message----- >> > > From: dts On Behalf Of Lijuan Tu >> > > Sent: Thursday, March 11, 2021 12:48 PM >> > > To: dpdklab@iol.unh.edu >> > > Cc: dts@dpdk.org; Lijuan Tu >> > > Subject: [dts] [PATCH] tests/nic_single_core_perf: modify >> > > accepted_tolerance to percentage >> > > >> > > * accepted_tolerance in conf/nic_single_core_perf.cfg are changed to >> > > percentage, e.g. >> > > accepted_tolerance =3D '1%%' >> > > if not set, it will be set to 2% by default. >> > > * delat in json are chagned to percentage too, e.g. >> > > 'delta': '1.5%' >> > > * expected_throughput will be forced to 1Mpps for compute delat in >> > > percentage >> > > * add new conf test_interval in conf/nic_single_core_perf.cfg, which >> > > indicates interval of getting throughput statistics in second, e.g. >> > > test_interval =3D 10 >> > > if not set, it will be set to 5 seconds by default. >> > > * collect better statistics , get multiple throughput statistics >> > > during the test run, and compute the average as the final results. >> > > >> > > Signed-off-by: Lijuan Tu >> > > --- >> > >> > > +accepted_tolerance =3D '1%%' >> > >> > > + diff =3D >> > > + (self.throughput[fwd_config][frame_size][nb_desc] - >> > > + >> > > self.expected_throughput[fwd_config][frame_size][nb_desc])/ \ >> > > + >> > > self.expected_throughput[fwd_config][frame_size][nb_desc] >> > >> > The value of `diff` here is still a decimal, and needs to be multiplie= d >> by a 100 >> > before it can be compared to `accepted_tolerance`. >> > >> > I suggest splitting this patch into multiple ones. One for making the >> percentage >> > change and one for taking the mean for multiple readings since they ar= e >> > unrelated changes. >> > I also see other unrelated changes not explained in the commit message= . >> >> Really appreciate for your review, I will accept your advices. >> >> > >> > Thanks, >> > Ali >> > --000000000000b31e3205be37744f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello Lijuan,

Would it be possible to h= ave the delta be a float in the json representation? For example, a 20% del= ta would be encoded as "delta": .2 in json. Currently, most of th= e community lab's=C2=A0infrastructure=C2=A0expects to make pass/fail de= terminations on a numeric type, so having the delta as a float would reduce= the required downtime to change everything over to percentages since we sh= ouldn't need to patch all of our services to expect a string.=C2=A0

Thanks,
Owen Hilyard

On Tue, Mar 2= 3, 2021 at 10:53 AM David Liu <dliu@= iol.unh.edu> wrote:
Thank you for Ali and Lijuan.

We had tes= ted the patch on the Arm machines setup. Other than the value issues, the t= est is running=C2=A0as expected.

On Tue, Mar 23, 2021 at 4:48 AM Tu, Lij= uan <lijuan.tu@= intel.com> wrote:


> -----Original Message-----
> From: Ali Alnubani <alialnu@nvidia.com>
> Sent: 2021=E5=B9=B43=E6=9C=8823=E6=97=A5 16:44
> To: Tu, Lijuan <lijuan.tu@intel.com>
> Cc: dts@dpdk.org= ; dpdklab@iol.unh.= edu
> Subject: RE: [dts] [PATCH] tests/nic_single_core_perf: modify
> accepted_tolerance to percentage
>
> > -----Original Message-----
> > From: dts <dts-bounces@dpdk.org> On Behalf Of Lijuan Tu
> > Sent: Thursday, March 11, 2021 12:48 PM
> > To: dpdk= lab@iol.unh.edu
> > Cc: dts@dpdk.or= g; Lijuan Tu <lijuan.tu@intel.com>
> > Subject: [dts] [PATCH] tests/nic_single_core_perf: modify
> > accepted_tolerance to percentage
> >
> > * accepted_tolerance in conf/nic_single_core_perf.cfg are changed= to
> > percentage, e.g.
> >=C2=A0 =C2=A0 =C2=A0accepted_tolerance =3D '1%%'
> >=C2=A0 =C2=A0 =C2=A0if not set, it will be set to 2% by default. > > * delat in json are chagned to percentage too, e.g.
> >=C2=A0 =C2=A0 =C2=A0'delta': '1.5%'
> > * expected_throughput will be forced to 1Mpps for compute delat i= n
> > percentage
> > * add new conf test_interval in conf/nic_single_core_perf.cfg, wh= ich
> > indicates interval of getting throughput statistics in second, e.= g.
> >=C2=A0 =C2=A0 =C2=A0test_interval =3D 10
> >=C2=A0 =C2=A0 =C2=A0if not set, it will be set to 5 seconds by def= ault.
> > * collect better statistics , get multiple throughput statistics<= br> > > during the test run, and compute the average as the final results= .
> >
> > Signed-off-by: Lijuan Tu <lijuan.tu@intel.com>
> > ---
> <snipped>
> > +accepted_tolerance =3D '1%%'
> <snipped>
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 diff =3D
> > + (self.throughput[fwd_config][frame_size][nb_desc] -
> > +
> > self.expected_throughput[fwd_config][frame_size][nb_desc])/ \
> > +
> > self.expected_throughput[fwd_config][frame_size][nb_desc]
>
> The value of `diff` here is still a decimal, and needs to be multiplie= d by a 100
> before it can be compared to `accepted_tolerance`.
>
> I suggest splitting this patch into multiple ones. One for making the = percentage
> change and one for taking the mean for multiple readings since they ar= e
> unrelated changes.
> I also see other unrelated changes not explained in the commit message= .

Really appreciate for your review, I will accept your advices.

>
> Thanks,
> Ali
--000000000000b31e3205be37744f--