Add to result heaps during profile (#503)

* Moved preamble for compile_and_sort into __init__

* Adding to heaps during profile

* Fixing L0 issues

* Renaming add_run_config_measurement to _add_rcm_to_results

* Moving add_rcm from model to metrics

* Getting closer. Still have unit tests failing

* All unit tests passing

* Refactoring __init__

* Fixing L0_state_manager

* Creating empty model list if calling result_manager from report

* Revert change

* Fixing minor nits

* Refactoring init

* Fixing report manager unit test

* Fixing result manager unit test

* Add public get methods to RCRC

* Adding back in exception if model is not found in checkpoint

* Adding in exception for models present in checkpoint. Updated report manager units tests

* Mocked check for models. All unit tests passing

* Fixing directory check issue in report manager unit testing

* Creating and removing test directory

* Fixing report manager unit testing the right way!

* Fixes based on Tim's review

* Removing uneeded method

* Changing Mocks to be CCRs

Author: nv-braf

model_analyzer/analyzer.py

@@ -113,9 +113,6 @@ def profile(self, client: TritonClient, gpus: List[GPUDevice], mode: str,
         logger.info("")
 
         if not self._config.skip_summary_reports:
-            # TODO: TMA-792: This won't be needed once the Results class is used in profile
-            self._analyze_models()
-
             self._create_summary_tables(verbose)
             self._create_summary_reports(mode)
             logger.info(self._get_report_command_help_string())
@@ -150,7 +147,6 @@ def analyze(self, mode: str, verbose: bool):
                                                         self._result_manager)
 
         # Create result tables, put top results and get stats
-        self._result_manager.compile_and_sort_results()
         self._report_manager.create_summaries()
         self._report_manager.export_summaries()
 
@@ -252,13 +248,6 @@ def _profile_models(self):
                 finally:
                     self._state_manager.save_checkpoint()
 
-    def _analyze_models(self):
-        # TODO: TMA-792: Until we get rid of analysis we need to copy some values from profile
-        self._config._fields["analysis_models"] = self._config._fields[
-            "profile_models"]
-
-        self._result_manager.compile_and_sort_results()
-
     def _create_summary_tables(self, verbose: bool):
         self._result_table_manager = ResultTableManager(self._config,
                                                         self._result_manager)

model_analyzer/result/result_manager.py

@@ -19,9 +19,14 @@
 
 from .result_heap import ResultHeap
 from .run_config_result_comparator import RunConfigResultComparator
+from .run_config_measurement import RunConfigMeasurement
 from .run_config_result import RunConfigResult
 from .results import Results
 
+from model_analyzer.config.input.config_command_profile import ConfigCommandProfile
+from model_analyzer.config.input.config_command_analyze import ConfigCommandAnalyze
+from model_analyzer.config.input.config_command_report import ConfigCommandReport
+
 from collections import defaultdict
 
 
@@ -35,7 +40,7 @@ def __init__(self, config, state_manager):
         """
         Parameters
         ----------
-        config :ConfigCommandProfile
+        config :ConfigCommandProfile/ConfigCommandReport
             the model analyzer config
         state_manager: AnalyzerStateManager
             The object that allows control and update of state
@@ -44,13 +49,15 @@ def __init__(self, config, state_manager):
         self._config = config
         self._state_manager = state_manager
 
-        if state_manager.starting_fresh_run():
-            self._init_state()
-
         # Data structures for sorting results
         self._per_model_sorted_results = defaultdict(ResultHeap)
         self._across_model_sorted_results = ResultHeap()
 
+        if state_manager.starting_fresh_run():
+            self._init_state()
+
+        self._complete_setup()
+
     def get_model_names(self):
         """
         Returns a list of model names that have sorted results
@@ -98,46 +105,31 @@ def add_server_data(self, data):
         self._state_manager.set_state_variable('ResultManager.server_only_data',
                                                data)
 
-    def add_run_config_measurement(self, run_config, run_config_measurement):
+    def add_run_config_measurement(
+            self, run_config, run_config_measurement: RunConfigMeasurement):
         """
-        This function adds model inference
-        measurements to the required result
-
-        Parameters
-        ----------
-        run_config : RunConfig
-            Contains the parameters used to generate the measurment
-        run_config_measurement: RunConfigMeasurement
-            the measurement to be added
+        Add measurement to individual result heap,
+        global result heap and results class
         """
+        model_name = run_config.models_name()
 
-        # Get reference to results state and modify it
-        results = self._state_manager.get_state_variable(
-            'ResultManager.results')
-
-        results.add_run_config_measurement(run_config, run_config_measurement)
-
-        # Use set_state_variable to record that state may have been changed
-        self._state_manager.set_state_variable(name='ResultManager.results',
-                                               value=results)
+        run_config_result = RunConfigResult(
+            model_name=model_name,
+            run_config=run_config,
+            comparator=self._run_comparators[model_name],
+            constraints=self._run_constraints[model_name])
 
-    def compile_and_sort_results(self):
-        """
-        Collects objectives and constraints for
-        each model, constructs results from the
-        measurements obtained, and sorts and 
-        filters them according to constraints
-        and objectives.
-        """
+        run_config_measurement.set_metric_weightings(
+            self._run_comparators[model_name].get_metric_weights())
 
-        self._create_concurrent_analysis_model_name()
+        run_config_measurement.set_model_config_weighting(
+            self._run_comparators[model_name].get_model_weights())
 
-        if self._analyzing_models_concurrently():
-            self._setup_for_concurrent_analysis()
-        else:
-            self._setup_for_sequential_analysis()
+        self._add_rcm_to_results(run_config, run_config_measurement)
+        run_config_result.add_run_config_measurement(run_config_measurement)
 
-        self._add_results_to_heaps()
+        self._per_model_sorted_results[model_name].add_result(run_config_result)
+        self._across_model_sorted_results.add_result(run_config_result)
 
     def get_model_configs_run_config_measurements(self, model_variants_name):
         """
@@ -252,6 +244,44 @@ def _init_state(self):
         self._state_manager.set_state_variable('ResultManager.server_only_data',
                                                {})
 
+    def _complete_setup(self):
+        # The Report subcommand can init, but nothing needs to be done
+        if isinstance(self._config, ConfigCommandProfile):
+            self._complete_profile_setup()
+        elif isinstance(self._config, ConfigCommandAnalyze):
+            self._complete_analyze_setup()
+        elif isinstance(self._config, ConfigCommandReport):
+            pass
+        else:
+            raise TritonModelAnalyzerException(
+                f"Expected config of type ConfigCommandProfile/ConfigCommandAnalyze/ConfigCommandReport,"
+                f" got {type(self._config)}.")
+
+    def _complete_profile_setup(self):
+        #TODO: TMA-792: Until we get rid of analysis we need this
+        self._config._fields["analysis_models"] = self._config._fields[
+            "profile_models"]
+
+        self._create_concurrent_analysis_model_name()
+
+        if self._config.run_config_profile_models_concurrently_enable:
+            self._setup_for_concurrent_analysis()
+        else:
+            self._setup_for_sequential_analysis()
+
+        self._add_results_to_heaps()
+
+    def _complete_analyze_setup(self):
+        self._create_concurrent_analysis_model_name()
+
+        if self._analyzing_models_concurrently():
+            self._setup_for_concurrent_analysis()
+        else:
+            self._setup_for_sequential_analysis()
+
+        self._check_for_models_in_checkpoint()
+        self._add_results_to_heaps()
+
     def _create_concurrent_analysis_model_name(self):
         analysis_model_names = [
             model.model_name() for model in self._config.analysis_models
@@ -310,6 +340,39 @@ def _setup_for_sequential_analysis(self):
             for model in self._config.analysis_models
         }
 
+    def _add_rcm_to_results(self, run_config, run_config_measurement):
+        """
+        This function adds model inference
+        measurements to the required result
+
+        Parameters
+        ----------
+        run_config : RunConfig
+            Contains the parameters used to generate the measurment
+        run_config_measurement: RunConfigMeasurement
+            the measurement to be added
+        """
+
+        # Get reference to results state and modify it
+        results = self._state_manager.get_state_variable(
+            'ResultManager.results')
+
+        results.add_run_config_measurement(run_config, run_config_measurement)
+
+        # Use set_state_variable to record that state may have been changed
+        self._state_manager.set_state_variable(name='ResultManager.results',
+                                               value=results)
+
+    def _check_for_models_in_checkpoint(self):
+        results = self._state_manager.get_state_variable(
+            'ResultManager.results')
+
+        for model_name in self._analysis_model_names:
+            if not results.get_model_measurements_dict(model_name):
+                raise TritonModelAnalyzerException(
+                    f"The model {model_name} was not found in the loaded checkpoint."
+                )
+
     def _add_results_to_heaps(self):
         """
         Construct and add results to individual result heaps 
@@ -321,10 +384,9 @@ def _add_results_to_heaps(self):
         for model_name in self._analysis_model_names:
             model_measurements = results.get_model_measurements_dict(model_name)
 
+            # Only add in models that exist in the checkpoint
             if not model_measurements:
-                raise TritonModelAnalyzerException(
-                    f"The model {model_name} was not found in the loaded checkpoint."
-                )
+                continue
 
             for (run_config,
                  run_config_measurements) in model_measurements.values():
@@ -336,10 +398,10 @@ def _add_results_to_heaps(self):
 
                 for run_config_measurement in run_config_measurements.values():
                     run_config_measurement.set_metric_weightings(
-                        self._run_comparators[model_name]._metric_weights)
+                        self._run_comparators[model_name].get_metric_weights())
 
                     run_config_measurement.set_model_config_weighting(
-                        self._run_comparators[model_name]._model_weights)
+                        self._run_comparators[model_name].get_model_weights())
 
                     run_config_result.add_run_config_measurement(
                         run_config_measurement)

model_analyzer/result/run_config_result_comparator.py

@@ -42,6 +42,12 @@ def __init__(self, metric_objectives_list):
             # TODO-TMA-571: Need to add support for model weighting
             self._model_weights.append(1)
 
+    def get_metric_weights(self):
+        return self._metric_weights
+
+    def get_model_weights(self):
+        return self._model_weights
+
     def is_better_than(self, run_config_result1, run_config_result2):
         """
         Aggregates and compares the score for two RunConfigResults 

tests/test_analyzer.py

@@ -57,19 +57,14 @@ def mock_get_state_variable(self, name):
     def mock_get_list_of_models(self):
         return ['model1']
 
-    @patch.multiple(
-        f'{AnalyzerStateManager.__module__}.AnalyzerStateManager',
-        get_state_variable=mock_get_state_variable,
-        exiting=lambda _: False
-    )
-    @patch.multiple(
-        f'{Analyzer.__module__}.Analyzer',
-        _create_metrics_manager=MagicMock(),
-        _create_model_manager=MagicMock(),
-        _get_server_only_metrics=MagicMock(),
-        _analyze_models=MagicMock(),
-        _profile_models=MagicMock()
-    )
+    @patch.multiple(f'{AnalyzerStateManager.__module__}.AnalyzerStateManager',
+                    get_state_variable=mock_get_state_variable,
+                    exiting=lambda _: False)
+    @patch.multiple(f'{Analyzer.__module__}.Analyzer',
+                    _create_metrics_manager=MagicMock(),
+                    _create_model_manager=MagicMock(),
+                    _get_server_only_metrics=MagicMock(),
+                    _profile_models=MagicMock())
     def test_profile_skip_summary_reports(self, **mocks):
         """
         Tests when the skip_summary_reports config option is turned on,
@@ -81,18 +76,16 @@ def test_profile_skip_summary_reports(self, **mocks):
         args = [
             'model-analyzer', 'profile', '--model-repository', '/tmp',
             '--profile-models', 'model1', '--config-file', '/tmp/my_config.yml',
-            '--checkpoint-directory', '/tmp/my_checkpoints', '--skip-summary-reports'
+            '--checkpoint-directory', '/tmp/my_checkpoints',
+            '--skip-summary-reports'
         ]
         config = evaluate_mock_config(args, '', subcommand="profile")
         state_manager = AnalyzerStateManager(config, None)
         analyzer = Analyzer(config,
                             None,
                             state_manager,
                             checkpoint_required=False)
-        analyzer.profile(client=None,
-                         gpus=None,
-                         mode=None,
-                         verbose=False)
+        analyzer.profile(client=None, gpus=None, mode=None, verbose=False)
 
         path = os.getcwd()
         self.assertFalse(os.path.exists(os.path.join(path, "plots")))
@@ -155,6 +148,9 @@ def mock_top_n_results(self, model_name=None, n=-1):
             RunConfigResult("fake_model_name", rc3, MagicMock())
         ]
 
+    def mock_check_for_models_in_checkpoint(self):
+        return True
+
     @patch(
         'model_analyzer.config.input.config_command_analyze.file_path_validator',
         lambda _: ConfigStatus(status=CONFIG_PARSER_SUCCESS))
@@ -163,6 +159,9 @@ def mock_top_n_results(self, model_name=None, n=-1):
         lambda _: None)
     @patch('model_analyzer.result.result_manager.ResultManager.top_n_results',
            mock_top_n_results)
+    @patch(
+        'model_analyzer.result.result_manager.ResultManager._check_for_models_in_checkpoint',
+        mock_check_for_models_in_checkpoint)
     def test_get_report_command_help_string(self):
         """
         Tests that the member function returning the report command help string

tests/test_plot_manager.py

@@ -103,8 +103,6 @@ def _create_single_model_result_manager(self):
 
         self._single_model_config = config
 
-        self._single_model_result_manager.compile_and_sort_results()
-
     def _create_multi_model_result_manager(self):
         args = [
             'model-analyzer', 'analyze', '-f', 'config.yml',
@@ -123,8 +121,6 @@ def _create_multi_model_result_manager(self):
 
         self._multi_model_config = config
 
-        self._multi_model_result_manager.compile_and_sort_results()
-
     def _plot_manager_to_dict(self, plot_manager):
         plot_manager_dict = {}
         plot_manager_dict['_simple_plots'] = {}