[Refactor] Rewrite scripts using pylint too-many-* suggestions (#3839)

Signed-off-by: Anatoly Myachev <[email protected]>

Author: anmyachev

.pre-commit-config.yaml

@@ -79,11 +79,6 @@ repos:
           - --disable=missing-module-docstring
           - --disable=missing-function-docstring
           - --disable=missing-class-docstring
-          - --disable=duplicate-code
-          - --disable=too-many-locals
-          - --disable=too-many-statements
-          - --disable=too-many-arguments
-          - --disable=too-many-positional-arguments
         stages: [pre-commit, pre-push, manual]
 
       - id: pylint
@@ -106,7 +101,6 @@ repos:
           - --disable=unnecessary-lambda-assignment
           # FIXME: revisit these checks later
           - --disable=too-few-public-methods
-          - --disable=consider-using-generator
           - --disable=missing-module-docstring
           - --disable=missing-function-docstring
           - --disable=missing-class-docstring

benchmarks/triton_kernels_benchmark/benchmark_testing.py

@@ -159,7 +159,7 @@ def extract_kernels(funcs):
         f"the profiling number not match; {n_repeat=}, {kernels=}, \n" +
         f"top functions by xpu_time:\n {prof.key_averages(group_by_stack_n=5).table(sort_by='xpu_time')}")
     # Make the time to the milliseconds.
-    times = torch.tensor([sum([k.duration for k in ks]) * 1e-3 for ks in kernels], dtype=torch.float)
+    times = torch.tensor([sum((k.duration for k in ks)) * 1e-3 for ks in kernels], dtype=torch.float)
     return _summarize_statistics(times, quantiles, return_mode)
 
 

scripts/build_report.py

@@ -4,11 +4,25 @@
 import uuid
 import json
 import datetime
+from dataclasses import dataclass
 
 import pandas as pd
 
 
-def parse_args():
+@dataclass
+class PassedArgs:  # pylint: disable=too-many-instance-attributes
+    source: str
+    target: str
+    param_cols: str
+    benchmark: str
+    compiler: str
+    tflops_col: str
+    hbm_col: str
+    tag: str
+    mask: bool
+
+
+def parse_args() -> PassedArgs:
     parser = argparse.ArgumentParser(description="Build report based on triton-benchmark run")
     parser.add_argument("source", help="Path to source csv file with benchmark results")
     parser.add_argument(
@@ -26,7 +40,8 @@ def parse_args():
     parser.add_argument("--hbm_col", help="Column name with HBM results.", required=False, default=None)
     parser.add_argument("--tag", help="How to tag results", required=False, default="")
     parser.add_argument("--mask", help="Mask identifiers among the params", required=False, action="store_true")
-    return parser.parse_args()
+    parsed_args = parser.parse_args()
+    return PassedArgs(**vars(parsed_args))
 
 
 def check_cols(target_cols, all_cols):
@@ -35,24 +50,26 @@ def check_cols(target_cols, all_cols):
         raise ValueError(f"Couldn't find required columns: '{diff}' among available '{all_cols}'")
 
 
-def transform_df(df, param_cols, tflops_col, hbm_col, benchmark, compiler, tag, mask):
+def transform_df(df, args: PassedArgs) -> pd.DataFrame:
+    param_cols = args.param_cols.split(",")
+    hbm_col = args.hbm_col
     check_cols(param_cols, df.columns)
-    check_cols([tflops_col] + [] if hbm_col is None else [hbm_col], df.columns)
+    check_cols([args.tflops_col] + [] if hbm_col is None else [hbm_col], df.columns)
     # Build json with parameters
     df_results = pd.DataFrame()
     # Type conversion to int is important here, because dashboards expect
     # int values.
     # Changing it without changing dashboards and database will
     # break comparison of old and new results
-    if mask:
+    if args.mask:
         df_results["MASK"] = df[param_cols[-1]]
         param_cols = param_cols[:-1]
         for p in param_cols:
             df[p] = df[p].astype(int)
             df_results["params"] = [json.dumps(j) for j in df[[*param_cols, "MASK"]].to_dict("records")]
     else:
         df_results["params"] = [json.dumps(j) for j in df[param_cols].astype(int).to_dict("records")]
-    df_results["tflops"] = df[tflops_col]
+    df_results["tflops"] = df[args.tflops_col]
     if hbm_col is not None:
         df_results["hbm_gbs"] = df[hbm_col]
 
@@ -70,9 +87,9 @@ def transform_df(df, param_cols, tflops_col, hbm_col, benchmark, compiler, tag,
         df_results["datetime"] = datetime.datetime.now()
     else:
         df_results["datetime"] = df["datetime"]
-    df_results["benchmark"] = benchmark
-    df_results["compiler"] = compiler
-    df_results["tag"] = tag
+    df_results["benchmark"] = args.benchmark
+    df_results["compiler"] = args.compiler
+    df_results["tag"] = args.tag
 
     host_info = {
         n: os.getenv(n.upper(), default="")
@@ -96,10 +113,8 @@ def transform_df(df, param_cols, tflops_col, hbm_col, benchmark, compiler, tag,
 
 def main():
     args = parse_args()
-    param_cols = args.param_cols.split(",")
     df = pd.read_csv(args.source)
-    result_df = transform_df(df, param_cols=param_cols, tflops_col=args.tflops_col, hbm_col=args.hbm_col,
-                             benchmark=args.benchmark, compiler=args.compiler, tag=args.tag, mask=args.mask)
+    result_df = transform_df(df, args)
     result_df.to_csv(args.target, index=False)
 
 

scripts/check_inductor_report.py

@@ -3,18 +3,35 @@
 from pathlib import Path
 import csv
 import sys
+from dataclasses import dataclass
 
 
-def check_report(suite, dtype, mode, test_mode, device, models_file, inductor_log_dir):
-    inductor_log_dir_leaf = Path(inductor_log_dir) / suite / dtype
-    inductor_report_filename = f"inductor_{suite}_{dtype}_{mode}_{device}_{test_mode}.csv"
-    inductor_report_path = Path(inductor_log_dir_leaf / inductor_report_filename)
+@dataclass
+class PassedArgs:
+    suite: str
+    dtype: str
+    mode: str
+    test_mode: str
+    device: str
+    models_file: str
+    inductor_log_dir: str
+
+
+def get_inductor_report_path(args: PassedArgs) -> Path:
+    inductor_log_dir_leaf = Path(args.inductor_log_dir) / args.suite / args.dtype
+    inductor_report_filename = f"inductor_{args.suite}_{args.dtype}_{args.mode}_{args.device}_{args.test_mode}.csv"
+    return Path(inductor_log_dir_leaf / inductor_report_filename)
+
+
+def check_report(args: PassedArgs) -> int:
+    test_mode = args.test_mode
+    inductor_report_path = get_inductor_report_path(args)
 
     subset = []
     report = []
     exitcode = 0
 
-    with open(models_file, encoding="utf-8") as f:
+    with open(args.models_file, encoding="utf-8") as f:
         subset = f.read().splitlines()
 
     with open(inductor_report_path, encoding="utf-8") as f:
@@ -23,7 +40,7 @@ def check_report(suite, dtype, mode, test_mode, device, models_file, inductor_lo
         for l in reader:
             report_with_header.append(l)
         for r in report_with_header[1:]:
-            if r[0] == device:
+            if r[0] == args.device:
                 report.append(r)
 
     test_list = [r[1] for r in report]
@@ -58,9 +75,9 @@ def main():
     argparser.add_argument("--device", help="i.e. xpu", required=True)
     argparser.add_argument("--models-file", help="Subset of models list", required=True)
     argparser.add_argument("--inductor-log-dir", help="Inductor test log directory", default="inductor_log")
-    args = argparser.parse_args()
-    exitcode = check_report(args.suite, args.dtype, args.mode, args.test_mode, args.device, args.models_file,
-                            args.inductor_log_dir)
+    parsed_args = argparser.parse_args()
+    passed_args = PassedArgs(**vars(parsed_args))
+    exitcode = check_report(passed_args)
     print(f"Report check result: {'SUCCESS' if exitcode == 0 else 'FAIL'}")
     sys.exit(exitcode)
 

scripts/compare-ci-runs/compare_runs.py

@@ -91,7 +91,7 @@ def parse_pytorch_benchmark_data(config: str, df: pd.DataFrame, file: Path) -> p
     raw_data["suite"] = suite
     raw_data["datatype"] = datatype
     raw_data["mode"] = mode
-    raw_data.rename(columns={"speedup": f"speedup {config}"}, inplace=True)
+    raw_data.rename(columns={"speedup": f"speedup-{config}"}, inplace=True)
 
     return pd.concat([df, raw_data], ignore_index=True)
 
@@ -146,9 +146,16 @@ def parse_directory(triton_benchmark: bool, config: str, directory: Path) -> pd.
     return df
 
 
-def summarize_diff(triton_benchmark: bool, perf_index: str, plot: bool, df: pd.DataFrame, num_col: str, denom_col: str,
-                   numerator: str, denominator: str):
+def get_column_names(perf_index: str, numerator: str, denominator: str):
+    num_col = f"{perf_index}-{numerator}"
+    denom_col = f"{perf_index}-{denominator}"
+    return num_col, denom_col
+
+
+def summarize_diff(perf_index: str, df: pd.DataFrame, numerator: str, denominator: str):
     """Summarize data difference of numerator and denominator."""
+    num_col, denom_col = get_column_names(perf_index, numerator, denominator)
+
     both_failed = df.loc[(df[num_col] == 0.0) & (df[denom_col] == 0.0)]
     print(f"Both failed ({both_failed.shape[0]} configurations):")
     print(both_failed.to_string())
@@ -195,54 +202,60 @@ def summarize_diff(triton_benchmark: bool, perf_index: str, plot: bool, df: pd.D
           f"{numerator}, showing relative difference in {perf_index})")
     print(df.head(print_cfgs))
     print("\n" * 2)
+    return df
 
-    if plot:
-        # pylint: disable=import-outside-toplevel
-        import seaborn as sns
-        import matplotlib.pyplot as plt
-        from matplotlib.backends.backend_pdf import PdfPages
 
-        keys = ["params", "benchmark"] if triton_benchmark else ["suite", "mode", "datatype"]
-        df["xlabel"] = df[keys].agg(", ".join, axis=1)
+def get_filename(perf_index, numerator, denominator) -> str:
+    num_col, denom_col = get_column_names(perf_index, numerator, denominator)
+    return f"performance-plot-{num_col}-{denom_col}.pdf".lower()
+
 
-        # Sort by configuration
-        order = list(df["xlabel"].unique())
-        order.sort()
-        filename = f"performance-plot-{num_col}-{denom_col}.pdf".lower()
-        with PdfPages(filename) as pdf:
-            fig = plt.figure()
-            plt.xticks(rotation=85)
+def plot_diff_df(df, triton_benchmark: bool, perf_index: str, numerator: str, denominator: str):
+    # pylint: disable=import-outside-toplevel
+    import seaborn as sns
+    import matplotlib.pyplot as plt
+    from matplotlib.backends.backend_pdf import PdfPages
 
-            title = ("Relative difference 0.0 means both perform identically,\n"
-                     f"relative difference > 0.0 means {numerator} performs better,\n"
-                     f"relative difference < 0.0 means {denominator} performs better")
-            plt.title(f"Comparison {numerator} vs {denominator}.")
+    keys = ["params", "benchmark"] if triton_benchmark else ["suite", "mode", "datatype"]
+    df["xlabel"] = df[keys].agg(", ".join, axis=1)
 
-            plt.figtext(1, 0.5, title)
+    # Sort by configuration
+    order = list(df["xlabel"].unique())
+    order.sort()
 
-            ax = sns.boxplot(df, x="xlabel", y="relative difference", order=order)
+    filename = get_filename(perf_index, numerator, denominator)
+    with PdfPages(filename) as pdf:
+        fig = plt.figure()
+        plt.xticks(rotation=85)
 
-            ax.set(xlabel=None, ylabel=f"Relative difference in {perf_index}")
+        title = ("Relative difference 0.0 means both perform identically,\n"
+                 f"relative difference > 0.0 means {numerator} performs better,\n"
+                 f"relative difference < 0.0 means {denominator} performs better")
+        plt.title(f"Comparison {numerator} vs {denominator}.")
 
-            pdf.savefig(fig, bbox_inches="tight")
-            print(f"Saved performance plot to {filename}")
+        plt.figtext(1, 0.5, title)
+
+        ax = sns.boxplot(df, x="xlabel", y="relative difference", order=order)
+
+        ax.set(xlabel=None, ylabel=f"Relative difference in {perf_index}")
+
+        pdf.savefig(fig, bbox_inches="tight")
+        print(f"Saved performance plot to {filename}")
 
 
 def eval_data(triton_benchmark: bool, plot: bool, df: pd.DataFrame, numerator: str, denominator: str):
     """Evaluate the data, print a summary and plot if enabled."""
     if triton_benchmark:
-        num_tri2xe_col = f"Tri2Xe-{numerator}"
-        dem_tri2xe_col = f"Tri2Xe-{denominator}"
-
-        df_ratio = df[["params", "benchmark", num_tri2xe_col, dem_tri2xe_col]]
-        summarize_diff(triton_benchmark, "tri2xe", plot, df_ratio, num_tri2xe_col, dem_tri2xe_col, numerator,
-                       denominator)
+        perf_index = "Tri2Xe"
+        num_col, denom_col = get_column_names(perf_index, numerator, denominator)
+        df_ratio = df[["params", "benchmark", num_col, denom_col]]
+        diff_df = summarize_diff(perf_index, df_ratio, numerator, denominator)
     else:
-        num_col = f"speedup {numerator}"
-        denom_col = f"speedup {denominator}"
-
+        perf_index = "speedup"
         df.drop(columns=["batch_size_x", "batch_size_y"], inplace=True)
-        summarize_diff(triton_benchmark, "speedup", plot, df, num_col, denom_col, numerator, denominator)
+        diff_df = summarize_diff(perf_index, df, numerator, denominator)
+    if plot:
+        plot_diff_df(diff_df, triton_benchmark, perf_index, numerator, denominator)
 
 
 def main():
@@ -295,8 +308,8 @@ def main():
             ]
         else:
             cols = [
-                "dev", "suite", "name", "mode", "datatype", "batch_size_x", "batch_size_y", f"speedup {num_cfg}",
-                f"speedup {denom_cfg}"
+                "dev", "suite", "name", "mode", "datatype", "batch_size_x", "batch_size_y", f"speedup-{num_cfg}",
+                f"speedup-{denom_cfg}"
             ]
 
         df = df[cols]