[Fuego] [PATCH] common: add new criteria operator bt
Tim.Bird at sony.com
Tim.Bird at sony.com
Thu Sep 27 18:34:35 UTC 2018
> -----Original Message-----
> From: Qiu Tingting
>
> Add new criteria operator bt to support that criteria check result is pass when
> ouput value is between min value and max value.
> e.g. of criteria file
> "reference":{
> "value":"7.5-10.5",
'-' is not a good range delimiter here, because it could be used as part
of a number value (for a negative number).
Possibly ',' would work, but this patch is quite involved and I didn't have
time to test it.
See more explanation below.
> "operator":"bt"
> }
>
> Signed-off-by: Qiu Tingting <qiutt at cn.fujitsu.com>
> ---
> engine/scripts/parser/common.py | 20 +++++---
> engine/scripts/parser/prepare_chart_data.py | 79
> +++++++++++++++++++++--------
> 2 files changed, 69 insertions(+), 30 deletions(-)
>
> diff --git a/engine/scripts/parser/common.py
> b/engine/scripts/parser/common.py
> index 9f3a8bb..5bea7f6 100644
> --- a/engine/scripts/parser/common.py
> +++ b/engine/scripts/parser/common.py
> @@ -158,17 +158,19 @@ def check_measure(tguid, criteria_data, measure):
> return 'SKIP'
>
> if reference['operator'] == 'lt':
> - result = measure['measure'] < reference["value"]
> + result = measure['measure'] < float(reference["value"])
> elif reference['operator'] == 'le':
> - result = measure['measure'] <= reference["value"]
> + result = measure['measure'] <= float(reference["value"])
> elif reference['operator'] == 'gt':
> - result = measure['measure'] > reference["value"]
> + result = measure['measure'] > float(reference["value"])
> elif reference['operator'] == 'ge':
> - result = measure['measure'] >= reference["value"]
> + result = measure['measure'] >= float(reference["value"])
> elif reference['operator'] == 'eq':
> - result = measure['measure'] == reference["value"]
> + result = measure['measure'] == float(reference["value"])
> elif reference['operator'] == 'ne':
> - result = measure['measure'] != reference["value"]
> + result = measure['measure'] != float(reference["value"])
> + elif reference['operator'] == 'bt':
> + result = measure['measure'] >= float(reference["value"].split('-')[0])
> and measure['measure'] <= float(reference["value"].split('-')[1])
splitting on '-' I problematical, because it will cause problems with negative numbers.
For example, this code will fail with the range "-7--3"
I changed the range delimiter to ',' so the above could be expressed as:
"-7,-3"
> status = 'PASS' if result else 'FAIL'
>
> dprint(" result=%s" % result)
> @@ -283,7 +285,7 @@ def convert_reference_log_to_criteria(filename):
> old_measure_id, operation = crit_str.split("|")
> ts_name, tc_name, measure_name = split_old_id(old_measure_id)
> tguid = "%s.%s.%s" % (ts_name, tc_name, measure_name)
> - value = float(lines[i+1])
> + value = lines[i+1]
>
> reference = {"value":value, "operator": operation.strip()}
> crit = {"tguid":tguid, "reference": reference }
> @@ -568,15 +570,17 @@ def process(results={}):
> update_results_json(test_logdir, TESTDIR, REF_JSON)
>
> ref_map = {}
> + op_map = {}
> for crit in criteria_data["criteria"]:
> # not all criteria are measure reference thresholds
> try:
> ref_map[crit["tguid"]] = crit["reference"]["value"]
> + op_map[crit["tguid"]] = crit["reference"]["operator"]
> except:
> # ignore other data
> pass
>
> - data_lines = store_flat_results(test_logdir, run_data, ref_map)
> + data_lines = store_flat_results(test_logdir, run_data, ref_map, op_map)
> make_chart_data(test_logdir, TESTDIR, CHART_CONFIG_JSON, data_lines)
>
> status = run_data.get('status', 'FAIL')
> diff --git a/engine/scripts/parser/prepare_chart_data.py
> b/engine/scripts/parser/prepare_chart_data.py
> index 4bf23cd..87af7fa 100644
> --- a/engine/scripts/parser/prepare_chart_data.py
> +++ b/engine/scripts/parser/prepare_chart_data.py
> @@ -66,9 +66,10 @@ class flat_entry:
> self.tguid = \
> self.test_set = \
> self.ref = \
> + self.op = \
> self.result = "ERROR-undefined"
> parts = line.split()
> - if len(parts) != 9:
> + if len(parts) < 9:
> print("Error reading line '%s' in prepare_chart_data.py" % line)
> print("Possible data corruption in flat_plot_data.txt")
>
> @@ -81,6 +82,7 @@ class flat_entry:
> self.kernel = parts[5]
> self.tguid = parts[6]
> self.ref = parts[7]
> + self.op = parts[9]
> self.result = parts[8]
>
> # break apart tguid
> @@ -103,10 +105,10 @@ class flat_entry:
> pass
>
> def __str__(self):
> - return "%s %s %s %s %s %s %s %s\n" % \
> + return "%s %s %s %s %s %s %s %s %s\n" % \
> (self.board, self.testname, self.spec, self.build_number,
> self.timestamp, self.kernel, self.tguid, self.ref,
> - self.result)
> + self.result, self.op)
>
> def get_chart_config(TESTDIR, chart_config_filename):
> try:
> @@ -146,10 +148,12 @@ def reread_run_json_files(TESTDIR):
> # and possibly FUEGO_CRITERIA_JSON_PATH)
> criteria_data = load_criteria()
> ref_map = {}
> + op_map = {}
> for crit in criteria_data["criteria"]:
> # not all criteria are measure reference thresholds
> try:
> ref_map[crit["tguid"]] = crit["reference"]["value"]
> + op_map[crit["tguid"]] = crit["reference"]["operator"]
> except:
> # ignore other data
> pass
> @@ -162,7 +166,7 @@ def reread_run_json_files(TESTDIR):
> # read run_data
> with open(run_filename) as run_file:
> run_data = json.load(run_file,
> object_pairs_hook=collections.OrderedDict)
> - store_flat_results(test_logdir, run_data, ref_map)
> + store_flat_results(test_logdir, run_data, ref_map, op_map)
>
> # Store tguid results in a flat list, space-separated
> # this is stored in a file called flat_plot_data.txt in the test directory
> @@ -173,7 +177,7 @@ def reread_run_json_files(TESTDIR):
> #
> # returns the list of lines in the updated flat results file
>
> -def store_flat_results(test_logdir, run_data, ref_map):
> +def store_flat_results(test_logdir, run_data, ref_map, op_map):
> # test log dir is up one directory
>
> plot_data_filename=test_logdir+"/flat_plot_data.txt"
> @@ -212,7 +216,7 @@ def store_flat_results(test_logdir, run_data,
> ref_map):
> return
>
> dprint("ref_map=%s" % ref_map)
> -
> + dprint("op_map=%s" % op_map)
> for test_set in test_sets:
> ts_name = test_set["name"]
> test_cases = test_set["test_cases"]
> @@ -245,11 +249,13 @@ def store_flat_results(test_logdir, run_data,
> ref_map):
> except:
> try:
> m_ref = ref_map[tguid]
> + m_op = op_map[tguid]
> except:
> m_ref = 0
> - line = "%s %s %s %s %s %s %s %s %s\n" % \
> + m_op = "none"
> + line = "%s %s %s %s %s %s %s %s %s %s\n" % \
OK - this is a lot of extra work and overhead (including the op for every
criteria reference, for every measure of every benchmark), to support
just a single operator ('bt') which might not be used that much.
It appears to me that this is only used one place in the make_measure_plot
code (the 'if entry.op == '"bt"'). Could you detect this automatically if
the ref value had a ',' in it? This would save having to extend the flat_plot_data
by an extra field, which is desirable.
> (board, test_name, test_spec, build_number, timestamp,
> - kernel, tguid, m_ref, m_result)
> + kernel, tguid, m_ref, m_result, m_op)
> plot_file.write(line)
> data_lines.append(line)
>
> @@ -319,20 +325,49 @@ def make_measure_plots(test_name,
> chart_config, entries):
>
> ref_series_key = series_key + "-ref"
>
> - dprint("ref_series_key=%s" % ref_series_key)
> -
> - if ref_series_key not in series_map:
> - dprint("making a new series for '%s'" % ref_series_key)
> - series_map[ref_series_key] = { "label": ref_series_key,
> - "data": [],
> - "points": {"symbol": "cross"} }
> - # add a plot point for this ref entry
> - try:
> - value = float(entry.ref)
> - except:
> - value = 0
> - point = [entry.build_number, value]
> - series_map[ref_series_key]["data"].append(point)
> + if entry.op == "bt":
Maybe detect this case with 'if ',' in entry.ref:' instead of adding
a whole field to flat_plot_data.txt
> + ref_min_series_key = ref_series_key + "-min"
> + dprint("ref_min_series_key=%s" % ref_min_series_key)
> + if ref_min_series_key not in series_map:
> + # add a plot point for this ref min entry
> + dprint("making a new series for '%s'" % ref_min_series_key)
> + series_map[ref_min_series_key] = { "label": ref_min_series_key,
> + "data": [],
> + "points": {"symbol": "cross"} }
> + try:
> + min_value = float(entry.ref.split('-')[0])
This would require changing also.
> + except:
> + min_value = 0
> + point = [entry.build_number, min_value]
> + series_map[ref_min_series_key]["data"].append(point)
> + try:
> + max_value = float(entry.ref.split('-')[1])
This would require changing also.
> + except:
> + max_value = None
> + ref_max_series_key = ref_series_key + "-max"
> + dprint("ref_max_series_key=%s" % ref_max_series_key)
> + if ref_max_series_key not in series_map:
> + # add a plot point for this ref max entry
> + dprint("making a new series for '%s'" % ref_max_series_key)
> + series_map[ref_max_series_key] = { "label":
> ref_max_series_key,
> + "data": [],
> + "points": {"symbol": "cross"} }
> + point = [entry.build_number, max_value]
> + series_map[ref_max_series_key]["data"].append(point)
> + else:
> + dprint("ref_series_key=%s" % ref_series_key)
> + if ref_series_key not in series_map:
> + # add a plot point for this ref entry
> + dprint("making a new series for '%s'" % ref_series_key)
> + series_map[ref_series_key] = { "label": ref_series_key,
> + "data": [],
> + "points": {"symbol": "cross"} }
> + try:
> + value = float(entry.ref)
> + except:
> + value = 0
> + point = [entry.build_number, value]
> + series_map[ref_series_key]["data"].append(point)
>
> flot_data = series_map.values()
> flot_data.sort(key=itemgetter('label'))
> --
> 2.7.4
OK - this is horrible of me, but I'm short on time. I applied the patch, and
changed the delimiter to ','. But I haven't had time to test it.
Can you please consider the change I recommend above, as well as
test this code, which I just pushed to master, to see if anything broke.
I also did a refactoring patch on top of this, for check_measure, to
make it a bit simpler.
Please take a look.
Thanks,
-- Tim
More information about the Fuego
mailing list