Merge branch 'master' into issue_2140

Author: namannandan

.github/workflows/benchmark_nightly.yml

@@ -65,6 +65,8 @@ jobs:
           if_no_artifact_found: ignore
           path: /tmp/ts_artifacts
           name: ${{ matrix.hardware }}_benchmark_validation
+      - name: Validate Benchmark result
+        run: python benchmarks/validate_report.py --input-artifacts-dir /tmp/ts_artifacts/${{ matrix.hardware }}_benchmark_validation
       - name: Update benchmark artifacts for auto validation
         run: python benchmarks/utils/update_artifacts.py --output /tmp/ts_artifacts/${{ matrix.hardware }}_benchmark_validation
       - name: Upload the updated benchmark artifacts for auto validation

.github/workflows/lint.yml

@@ -77,6 +77,7 @@ jobs:
           echo "cd serve/"
           echo "pre-commit install"
           echo "pre-commit will lint your code for you, so git add and commit those new changes and this check should become green"
+          echo "If you've already pushed some files remotely then run git diff --name-only main | xargs pre-commit run --files"
 
   spellcheck:
     runs-on: ubuntu-20.04

benchmarks/auto_benchmark.py

@@ -17,9 +17,10 @@
 
 
 class BenchmarkConfig:
-    def __init__(self, yaml_dict, skip_ts_install):
+    def __init__(self, yaml_dict, skip_ts_install, skip_upload):
         self.yaml_dict = yaml_dict
         self.skip_ts_install = skip_ts_install
+        self.skip_upload = skip_upload
         self.bm_config = {}
         yesterday = datetime.date.today() - datetime.timedelta(days=1)
         self.bm_config["version"] = "torchserve-nightly=={}.{}.{}".format(
@@ -89,9 +90,9 @@ def load_config(self):
                 self.models(v)
             elif k == "hardware":
                 self.hardware(v)
-            elif k == "metrics_cmd":
+            elif k == "metrics_cmd" and not self.skip_upload:
                 self.metrics_cmd(v)
-            elif k == "report_cmd":
+            elif k == "report_cmd" and not self.skip_upload:
                 report_cmd = v
 
         self.bm_config["model_config_path"] = (
@@ -110,12 +111,12 @@ def load_config(self):
             print("{}={}".format(k, v))
 
 
-def load_benchmark_config(bm_config_path, skip_ts_install):
+def load_benchmark_config(bm_config_path, skip_ts_install, skip_upload):
     yaml = ruamel.yaml.YAML()
     with open(bm_config_path, "r") as f:
         yaml_dict = yaml.load(f)
 
-        benchmark_config = BenchmarkConfig(yaml_dict, skip_ts_install)
+        benchmark_config = BenchmarkConfig(yaml_dict, skip_ts_install, skip_upload)
         benchmark_config.load_config()
 
     return benchmark_config.bm_config
@@ -285,14 +286,23 @@ def main():
         action="store",
         help="true: skip torchserve installation. default: true",
     )
+    parser.add_argument(
+        "--skip_upload",
+        help="true: skip uploading commands . default: false",
+    )
 
     arguments = parser.parse_args()
     skip_ts_config = (
         False
         if arguments.skip is not None and arguments.skip.lower() == "false"
         else True
     )
-    bm_config = load_benchmark_config(arguments.input, skip_ts_config)
+    skip_upload = (
+        True
+        if arguments.skip_upload is not None and arguments.skip_upload.lower() == "true"
+        else False
+    )
+    bm_config = load_benchmark_config(arguments.input, skip_ts_config, skip_upload)
     benchmark_env_setup(bm_config, skip_ts_config)
     run_benchmark(bm_config)
     clean_up_benchmark_env(bm_config)

benchmarks/utils/report.py

@@ -0,0 +1,75 @@
+import csv
+
+METRICS_VALIDATED = [
+    "TS throughput",
+    "TS latency P50",
+    "TS latency P90",
+    "TS latency P99",
+    "Model_p50",
+    "Model_p90",
+    "Model_p99",
+    "memory_percentage_mean",
+    "gpu_memory_used_mean",
+    "cpu_percentage_mean",
+    "gpu_percentage_mean",
+]
+
+
+# Acceptable metric deviation needs a more complicated logic.
+# Example: For latencies in 2 digits, 50% might be acceptable
+# For 3 digit latencies, 20-30% might be the right value
+# For cpu_memory < 15%, 50% deviation works but for CPU > 40%, 10-15%
+# might be the right value
+ACCEPTABLE_METRIC_DEVIATION = 0.3
+
+
+class Report:
+    def __init__(self, deviation=0, num_reports=0):
+        self.properties = {}
+        self.mode = None
+        self.throughput = 0
+        self.batch_size = 0
+        self.workers = 0
+        self.deviation = deviation
+        self.num_reports = num_reports
+
+    def _get_mode(self, csv_file):
+        cfg = csv_file.split("/")[-2]
+        cfg = cfg.split("_")
+        mode = cfg[0] + "_" + cfg[1]
+        self.mode = mode
+
+    def read_csv(self, csv_file):
+        with open(csv_file, newline="") as f:
+            reader = csv.DictReader(f)
+            for k, v in next(reader).items():
+                if k in METRICS_VALIDATED:
+                    self.properties[k] = float(v)
+        self._get_mode(csv_file)
+
+    def update(self, report):
+        for property in self.properties:
+            # sum the properties to find the mean later
+            self.properties[property] += report.properties[property]
+
+    def mean(self):
+        for k, v in self.properties.items():
+            self.properties[k] = v / self.num_reports
+
+
+def metric_valid(key, obs_val, exp_val, threshold):
+    # In case of throughput, higher is better
+    # In case of memory, lower is better.
+    # We ignore lower values for memory related metrices
+    lower = False
+    if "throughput" not in key:
+        lower = True
+    return check_if_within_threshold(exp_val, obs_val, threshold) or (
+        (obs_val < exp_val and lower)
+    )
+
+
+def check_if_within_threshold(value1, value2, threshold):
+    if float(value1) == 0.0:
+        return True
+    return abs((value1 - value2) / float(value1)) <= threshold

benchmarks/validate_report.py

@@ -0,0 +1,98 @@
+import argparse
+import os
+
+from utils.report import (
+    ACCEPTABLE_METRIC_DEVIATION,
+    METRICS_VALIDATED,
+    Report,
+    metric_valid,
+)
+from utils.update_artifacts import (
+    BENCHMARK_ARTIFACTS_PATH,
+    BENCHMARK_REPORT_FILE,
+    BENCHMARK_REPORT_PATH,
+)
+
+
+def validate_reports(artifacts_dir, report_dir, deviation):
+    # Read baseline reports
+    baseline_reports = {}
+    num_reports = len(os.listdir(artifacts_dir))
+    for _d in sorted(os.listdir(artifacts_dir)):
+        dir = os.path.join(artifacts_dir, _d)
+        for subdir in sorted(os.listdir(dir)):
+            csv_file = os.path.join(dir, subdir, BENCHMARK_REPORT_FILE)
+
+            report = Report(deviation, num_reports)
+            report.read_csv(csv_file)
+            if subdir not in baseline_reports:
+                baseline_reports[subdir] = report
+            else:
+                baseline_reports[subdir].update(report)
+
+    # Get the mean value each of the properties for every report
+    for model, report in baseline_reports.items():
+        report.mean()
+        baseline_reports[model] = report
+
+    # Read generated reports
+    generated_reports = {}
+    for subdir in sorted(os.listdir(report_dir)):
+        if os.path.isdir(os.path.join(report_dir, subdir)):
+            csv_file = os.path.join(report_dir, subdir, BENCHMARK_REPORT_FILE)
+            report = Report()
+            report.read_csv(csv_file)
+            generated_reports[subdir] = report
+
+    # Compare generated reports with baseline reports
+    error = False
+    for model, report in generated_reports.items():
+        for key in METRICS_VALIDATED:
+            if not metric_valid(
+                key,
+                report.properties[key],
+                baseline_reports[model].properties[key],
+                baseline_reports[model].deviation,
+            ):
+                print(
+                    f"Error while validating {key} for model: {model}, "
+                    f"Expected value: {baseline_reports[model].properties[key]:.2f}, "
+                    f"Observed value: {report.properties[key]:.2f}"
+                )
+                error = True
+        if not error:
+            print(f"Model {model} successfully validated")
+
+    if error:
+        raise Exception("Failures in benchmark validation")
+
+
+def main():
+    parser = argparse.ArgumentParser()
+
+    parser.add_argument(
+        "--input-artifacts-dir",
+        help="directory where benchmark artifacts have been saved",
+        type=str,
+        default=BENCHMARK_ARTIFACTS_PATH,
+    )
+
+    parser.add_argument(
+        "--input-report-dir",
+        help="directory where current benchmark report is saved",
+        type=str,
+        default=BENCHMARK_REPORT_PATH,
+    )
+
+    parser.add_argument(
+        "--deviation",
+        help="acceptable variation in metrics values ",
+        type=float,
+        default=ACCEPTABLE_METRIC_DEVIATION,
+    )
+    args = parser.parse_args()
+    validate_reports(args.input_artifacts_dir, args.input_report_dir, args.deviation)
+
+
+if __name__ == "__main__":
+    main()

docs/torchserve_on_win_native.md

@@ -11,7 +11,7 @@
  - At present, it has only been certified on windows server 2019 however should work fine on Windows 10.
  - Make sure you are an admin user or have admin rights
  - The instruction given here will use anaconda Powershell terminal to install torchserve
- - Install Anaconda as given [here](https://docs.anaconda.com/free/anaconda/getting-started/install/windows/)
+ - Install Anaconda as given [here](https://docs.anaconda.com/anaconda/install/windows/)
  - Install Git as given [here](https://github.com/git-for-windows/git/releases/download/v2.28.0.windows.1/Git-2.28.0-64-bit.exe)
  - Install openjdk17
     - Download [openjdk17](https://download.oracle.com/java/17/archive/jdk-17.0.3_windows-x64_bin.zip)

examples/pt2/README.md

@@ -23,8 +23,8 @@ As an example let's expand our getting started guide with the only difference be
 
 ```
 mkdir model_store
-torch-model-archiver --model-name densenet161 --version 1.0 --model-file ./serve/examples/image_classifier/densenet_161/model.py --export-path model_store --extra-files ./serve/examples/image_classifier/index_to_name.json --handler image_classifier
-torchserve --start --ncs --model-store model_store --models densenet161.mar --config-file model_config.yaml
+torch-model-archiver --model-name densenet161 --version 1.0 --model-file ./serve/examples/image_classifier/densenet_161/model.py --export-path model_store --extra-files ./serve/examples/image_classifier/index_to_name.json --handler image_classifier --config-file model_config.yaml
+torchserve --start --ncs --model-store model_store --models densenet161.mar
 ```
 
 The exact same approach works with any other model, what's going on is the below

kubernetes/kserve/kserve_wrapper/TorchserveModel.py

@@ -129,10 +129,5 @@ def load(self) -> bool:
         existing_paths = [path for path in paths if path.exists()]
         if len(existing_paths) == 0:
             raise ModelMissingError(model_path)
-        elif len(existing_paths) > 1:
-            raise RuntimeError(
-                "More than one model file is detected, "
-                f"Only one is allowed within model_dir: {existing_paths}"
-            )
         self.ready = True
         return self.ready