From 252dbe9c72f12d05f7d476f97d7fb97f73d0080c Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Tue, 26 Mar 2024 01:45:00 -0700 Subject: [PATCH] stdlib: Add tests for PyStats's Vector and fix bugs The big thing missing from the Vector stats was that each position in the vector could have it's own unique id (a str, float, or int) and each position in the vector can have its own description. Therefore, to add this the Vector is represented as a dictionary mapping the unique ID to a Pystat Scaler (whcih can have it's own unique description. Change-Id: I3a8634f43298f6491300cf5a4f9d25dee8101808 --- .../m5/ext/pystats/serializable_stat.py | 5 + src/python/m5/ext/pystats/statistic.py | 41 ++++-- src/python/m5/stats/gem5stats.py | 37 +++-- src/test_objects/SConscript | 6 +- src/test_objects/StatTester.py | 18 +++ src/test_objects/stat_tester.cc | 33 +++++ src/test_objects/stat_tester.hh | 22 +++ .../gem5/stats/configs/pystat_vector_check.py | 137 ++++++++++++++++++ .../stats/configs/simstat_output_check.py | 49 +++---- tests/gem5/stats/test_simstats_output.py | 77 +++++++++- 10 files changed, 365 insertions(+), 60 deletions(-) create mode 100644 tests/gem5/stats/configs/pystat_vector_check.py diff --git a/src/python/m5/ext/pystats/serializable_stat.py b/src/python/m5/ext/pystats/serializable_stat.py index faa70a5022..e4eb861d3f 100644 --- a/src/python/m5/ext/pystats/serializable_stat.py +++ b/src/python/m5/ext/pystats/serializable_stat.py @@ -85,6 +85,11 @@ class SerializableStat: return value elif isinstance(value, datetime): return value.replace(microsecond=0).isoformat() + elif isinstance(value, Dict): + d = {} + for k, v in value.items(): + d[self.__process_json_value(k)] = self.__process_json_value(v) + return d elif isinstance(value, list): return [self.__process_json_value(v) for v in value] elif isinstance(value, StorageType): diff --git a/src/python/m5/ext/pystats/statistic.py b/src/python/m5/ext/pystats/statistic.py index f11fc94a29..3fb4fc11f6 100644 --- a/src/python/m5/ext/pystats/statistic.py +++ b/src/python/m5/ext/pystats/statistic.py @@ -27,8 +27,8 @@ from abc import ABC from typing import ( Any, + Dict, Iterable, - List, Optional, Union, ) @@ -90,41 +90,56 @@ class Vector(Statistic): An Python statistics which representing a vector of Scalar values. """ - value: List[Union[int, float]] - def __init__( self, - value: Iterable[Union[int, float]], + value: Dict[Union[str, int, float], Scalar], type: Optional[str] = None, description: Optional[str] = None, ): super().__init__( - value=list(value), + value=value, type=type, description=description, ) + def __getitem__(self, index: Union[int, str, float]) -> Scalar: + assert self.value != None + # In the case of string, we cast strings to integers of floats if they + # are numeric. This avoids users having to cast strings to integers. + if isinstance(index, str): + if index.isindex(): + index = int(index) + elif index.isnumeric(): + index = float(index) + return self.value[index] + + def size(self) -> int: + """ + Returns the size of the vector. + + :returns: The size of the vector. + """ + assert self.value != None + return len(self.value) + def mean(self) -> float: """ Returns the mean of the value vector. - :returns: The mean value across all bins. + :returns: The mean value across all values in the vector. """ assert self.value != None - assert isinstance(self.value, List) - from statistics import mean as statistics_mean - - return statistics_mean(self.value) + return self.count() / self.size() def count(self) -> float: """ - Returns the count across all the bins. + Returns the count (sum) of all values in the vector. - :returns: The sum of all bin values. + :returns: The sum of all vector values. """ assert self.value != None - return sum(self.value) + return sum(float(self.value[key]) for key in self.values) class Distribution(Vector): diff --git a/src/python/m5/stats/gem5stats.py b/src/python/m5/stats/gem5stats.py index 8b619a00b0..5e0d57ec15 100644 --- a/src/python/m5/stats/gem5stats.py +++ b/src/python/m5/stats/gem5stats.py @@ -183,29 +183,44 @@ def __get_distribution(statistic: _m5.stats.DistInfo) -> Distribution: def __get_vector(statistic: _m5.stats.VectorInfo) -> Vector: - to_add = dict() + vec: Dict[Union[str, int, float], Scalar] = {} + for index in range(statistic.size): # All the values in a Vector are Scalar values value = statistic.value[index] - unit = statistic.unit - description = statistic.subdescs[index] - # ScalarInfo uses the C++ `double`. - datatype = StorageType["f64"] + assert isinstance(value, float) or isinstance(value, int) # Sometimes elements within a vector are defined by their name. Other # times they have no name. When a name is not available, we name the # stat the index value. - if str(statistic.subnames[index]): - index_string = str(statistic.subnames[index]) + if len(statistic.subnames) > index and statistic.subnames[index]: + index_subname = str(statistic.subnames[index]) + if index_subname.isdigit(): + index_subname = int(index_subname) + elif index_subname.isnumeric(): + index_subname = float(index_subname) else: - index_string = str(index) + index_subname = index - to_add[index_string] = Scalar( - value=value, unit=unit, description=description, datatype=datatype + index_subdesc = None + if len(statistic.subdescs) > index and statistic.subdescs[index]: + index_subdesc = str(statistic.subdescs[index]) + else: + index_subdesc = statistic.desc + + vec[index_subname] = Scalar( + value=value, + unit=statistic.unit, + description=index_subdesc, + datatype=StorageType["f64"], ) - return Vector(scalar_map=to_add) + return Vector( + vec, + type="Vector", + description=statistic.desc, + ) def _prepare_stats(group: _m5.stats.Group): diff --git a/src/test_objects/SConscript b/src/test_objects/SConscript index 6d1d1ec2d1..840a920af2 100644 --- a/src/test_objects/SConscript +++ b/src/test_objects/SConscript @@ -27,5 +27,9 @@ Import ("*") if env['CONF']['USE_TEST_OBJECTS']: - SimObject('StatTester.py', sim_objects=['StatTester', 'ScalarStatTester']) + SimObject('StatTester.py', sim_objects=[ + 'StatTester', + 'ScalarStatTester', + 'VectorStatTester', + ]) Source('stat_tester.cc') diff --git a/src/test_objects/StatTester.py b/src/test_objects/StatTester.py index a413b47764..9664ab92d5 100644 --- a/src/test_objects/StatTester.py +++ b/src/test_objects/StatTester.py @@ -44,3 +44,21 @@ class ScalarStatTester(StatTester): cxx_class = "gem5::ScalarStatTester" value = Param.Float("The scalar stat's value.") + + +class VectorStatTester(StatTester): + type = "VectorStatTester" + cxx_header = "test_objects/stat_tester.hh" + cxx_class = "gem5::VectorStatTester" + + values = VectorParam.Float("The vector stat's values.") + subnames = VectorParam.String( + [], + "The vector stat's subnames. If position is empty, index int is " + "used instead.", + ) + subdescs = VectorParam.String( + [], + "The vector stat's subdescriptions. If empty, the subdescriptions " + "are not used.", + ) diff --git a/src/test_objects/stat_tester.cc b/src/test_objects/stat_tester.cc index bf90993287..5a52d6991f 100644 --- a/src/test_objects/stat_tester.cc +++ b/src/test_objects/stat_tester.cc @@ -51,4 +51,37 @@ ScalarStatTester::ScalarStatTesterStats::ScalarStatTesterStats( { } +void +VectorStatTester::setStats() +{ + for (int i = 0; i < params.values.size(); i++) + { + stats.vector[i] = (params.values[i]); + } +} + +VectorStatTester::VectorStatTesterStats::VectorStatTesterStats( + statistics::Group *parent, + const VectorStatTesterParams ¶ms +) : statistics::Group(parent), + vector(this, + params.name.c_str(), + statistics::units::Count::get(), + params.description.c_str() + ) +{ + vector.init(params.values.size()); + for (int i = 0; i < params.values.size(); i++) + { + if (params.subnames.size() > i) { + vector.subname(i, params.subnames[i]); + } else { + vector.subname(i, std::to_string(i)); + } + if (params.subdescs.size() > i) { + vector.subdesc(i, params.subdescs[i]); + } + } +} + } // namespace gem5 diff --git a/src/test_objects/stat_tester.hh b/src/test_objects/stat_tester.hh index d9e3371997..d2099583f6 100644 --- a/src/test_objects/stat_tester.hh +++ b/src/test_objects/stat_tester.hh @@ -32,6 +32,7 @@ #include "base/statistics.hh" #include "params/ScalarStatTester.hh" #include "params/StatTester.hh" +#include "params/VectorStatTester.hh" #include "sim/sim_object.hh" namespace gem5 @@ -114,6 +115,27 @@ class ScalarStatTester : public StatTester } stats; }; +class VectorStatTester : public StatTester +{ + private: + VectorStatTesterParams params; + + public: + VectorStatTester(const VectorStatTesterParams &p) : + StatTester(p), params(p), stats(this, p) {} + + protected: + void setStats() override; + struct VectorStatTesterStats : public statistics::Group + { + VectorStatTesterStats( + statistics::Group *parent, + const VectorStatTesterParams ¶ms + ); + statistics::Vector vector; + } stats; +}; + } // namespace gem5 #endif // __STAT_TESTER_HH__ diff --git a/tests/gem5/stats/configs/pystat_vector_check.py b/tests/gem5/stats/configs/pystat_vector_check.py new file mode 100644 index 0000000000..65f847b28c --- /dev/null +++ b/tests/gem5/stats/configs/pystat_vector_check.py @@ -0,0 +1,137 @@ +# Copyright (c) 2024 The Regents of the University of California +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer; +# redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution; +# neither the name of the copyright holders nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import argparse +import sys + +import m5 +from m5.objects import ( + Root, + VectorStatTester, +) +from m5.stats.gem5stats import get_simstat + +"""This script is used for checking that the Vector statistics set in " +the simulation are correctly parsed through to the python Pystats. +""" + +parser = argparse.ArgumentParser( + description="Tests the output of a Vector PyStat." +) + +parser.add_argument( + "value", + help="Comma delimited list representing the vector.", + type=lambda s: [float(item) for item in s.split(",")], +) + +parser.add_argument( + "--name", + type=str, + default="vector", + required=False, + help="Name of the vector statistic.", +) + +parser.add_argument( + "--description", + type=str, + default="", + required=False, + help="Description of the vector statistic.", +) + +parser.add_argument( + "--subnames", + help="Comma delimited list representing the vector subnames.", + type=str, +) + +parser.add_argument( + "--subdescs", + help="Comma delimited list representing the vector subdescs", + type=str, +) + +args = parser.parse_args() + +stat_tester = VectorStatTester() +stat_tester.name = args.name +stat_tester.description = args.description +stat_tester.values = args.value + +stat_tester.subnames = [] +if args.subnames: + stat_tester.subnames = [str(item) for item in args.subnames.split(",")] + +stat_tester.subdescs = [] +if args.subdescs: + stat_tester.subdescs = [str(item) for item in args.subdescs.split(",")] + +value_dict = {} +for i in range(len(args.value)): + i_name = i + description = args.description + if stat_tester.subnames and i < len(stat_tester.subnames): + i_name = stat_tester.subnames[i] + if stat_tester.subdescs and i < len(stat_tester.subdescs): + description = stat_tester.subdescs[i] + + value_dict[i_name] = { + "value": args.value[i], + "type": "Scalar", + "unit": "Count", + "description": description, + "datatype": "f64", + } + +expected_output = { + "type": "Group", + "time_conversion": None, + args.name: { + "value": value_dict, + "type": "Vector", + "description": args.description, + }, +} + +root = Root(full_system=False, system=stat_tester) + +m5.instantiate() +m5.simulate() + +simstats = get_simstat(stat_tester) +output = simstats.to_json()["system"] + +if output != expected_output: + print("Output statistics do not match expected:", file=sys.stderr) + print("", file=sys.stderr) + print("Expected:", file=sys.stderr) + print(expected_output, file=sys.stderr) + print("", file=sys.stderr) + print("Actual:", file=sys.stderr) + print(output, file=sys.stderr) + sys.exit(1) diff --git a/tests/gem5/stats/configs/simstat_output_check.py b/tests/gem5/stats/configs/simstat_output_check.py index 4cbe2d0ca7..f794e63c48 100644 --- a/tests/gem5/stats/configs/simstat_output_check.py +++ b/tests/gem5/stats/configs/simstat_output_check.py @@ -26,7 +26,6 @@ import argparse import sys -from typing import Union import m5 from m5.objects import ( @@ -42,24 +41,18 @@ against the expected JSON output and that produced by the SimStats module. """ parser = argparse.ArgumentParser(description="Tests the output of a SimStat.") -subparsers = parser.add_subparsers( - dest="statistic", help="SimStats statistic to test", required=True -) -scalar_parser = subparsers.add_parser( - "scalar", help="Test a scalar statistic." -) -scalar_parser.add_argument( +parser.add_argument( "value", type=float, help="The value of the scalar statistic." ) -scalar_parser.add_argument( +parser.add_argument( "--name", type=str, default="scalar", required=False, help="The name of the scalar statistic.", ) -scalar_parser.add_argument( +parser.add_argument( "--description", type=str, default="", @@ -69,27 +62,21 @@ scalar_parser.add_argument( args = parser.parse_args() -expected_output = None -stat_tester = None -if args.statistic == "scalar": - stat_tester = ScalarStatTester() - stat_tester.name = args.name - stat_tester.description = args.description - stat_tester.value = args.value - expected_output = { - "type": "Group", - "time_conversion": None, - args.name: { - "value": args.value, - "type": "Scalar", - "unit": "Count", - "description": args.description, - "datatype": "f64", - }, - } - -assert stat_tester is not None -assert expected_output is not None +stat_tester = ScalarStatTester() +stat_tester.name = args.name +stat_tester.description = args.description +stat_tester.value = args.value +expected_output = { + "type": "Group", + "time_conversion": None, + args.name: { + "value": args.value, + "type": "Scalar", + "unit": "Count", + "description": args.description, + "datatype": "f64", + }, +} root = Root(full_system=False, system=stat_tester) diff --git a/tests/gem5/stats/test_simstats_output.py b/tests/gem5/stats/test_simstats_output.py index c7393ebf2b..7d3752f0bc 100644 --- a/tests/gem5/stats/test_simstats_output.py +++ b/tests/gem5/stats/test_simstats_output.py @@ -39,7 +39,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "42", "--name", "scalar_test", @@ -63,7 +62,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "0", ], valid_isas=(constants.all_compiled_tag,), @@ -83,7 +81,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "-245", ], valid_isas=(constants.all_compiled_tag,), @@ -103,7 +100,6 @@ gem5_verify_config( "simstat_output_check.py", ), config_args=[ - "scalar", "42.869", "--name", "float_test", @@ -113,3 +109,76 @@ gem5_verify_config( valid_isas=(constants.all_compiled_tag,), length=constants.quick_tag, ) + +gem5_verify_config( + name="pystat_vector_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector_check.py", + ), + config_args=[ + "2.0,4,5.9,2.3,-8,0,0.0,-8.9", + "--name", + "vector_stat", + "--description", + "A vector statistic with a float value", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) + +gem5_verify_config( + name="pystat_vector_with_subnames_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector_check.py", + ), + config_args=[ + "2.0,4,3", + "--name", + "vector_stat", + "--description", + "A vector statistic with a float value", + "--subnames", + "first,second,third", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +) + +gem5_verify_config( + name="pystat_vector_with_subdescs_test", + fixtures=(), + verifiers=[], + config=joinpath( + config.base_dir, + "tests", + "gem5", + "stats", + "configs", + "pystat_vector_check.py", + ), + config_args=[ + "2.0,4,3", + "--name", + "vector_stat", + "--description", + "A vector statistic with a float value", + "--subdescs", + "first,second", + ], + valid_isas=(constants.all_compiled_tag,), + length=constants.quick_tag, +)