diff --git a/CIME/Tools/bless_test_results b/CIME/Tools/bless_test_results index d2bc707d3c2..eb9663dcf9a 100755 --- a/CIME/Tools/bless_test_results +++ b/CIME/Tools/bless_test_results @@ -116,12 +116,22 @@ def create_bless_options(parser): "--hist-only", action="store_true", help="Only analyze history files." ) - mutual_bless_group.add_argument( - "--tput-only", action="store_true", help="Only analyze throughput." + mutual_perf_group = bless_group.add_mutually_exclusive_group() + + mutual_perf_group.add_argument( + "--bless-tput", + action="store_true", + help="Bless throughput, use `--bless-perf` to bless throughput and memory", ) - mutual_bless_group.add_argument( - "--mem-only", action="store_true", help="Only analyze memory." + mutual_perf_group.add_argument( + "--bless-mem", + action="store_true", + help="Bless memory, use `--bless-perf` to bless throughput and memory", + ) + + bless_group.add_argument( + "--bless-perf", action="store_true", help="Bless both throughput and memory" ) diff --git a/CIME/baselines/performance.py b/CIME/baselines/performance.py index 8aa76083a82..0fcb729d1d5 100644 --- a/CIME/baselines/performance.py +++ b/CIME/baselines/performance.py @@ -129,6 +129,8 @@ def perf_write_baseline(case, basegen_dir, throughput=True, memory=True): write_baseline_file(baseline_file, tput) + logger.info("Updated throughput baseline to {!s}".format(tput)) + if memory: try: mem = perf_get_memory(case, config) @@ -139,6 +141,8 @@ def perf_write_baseline(case, basegen_dir, throughput=True, memory=True): write_baseline_file(baseline_file, mem) + logger.info("Updated memory usage baseline to {!s}".format(mem)) + def load_coupler_customization(case): """ diff --git a/CIME/bless_test_results.py b/CIME/bless_test_results.py index 8817f1f22a4..c6f0754ebd8 100644 --- a/CIME/bless_test_results.py +++ b/CIME/bless_test_results.py @@ -22,7 +22,7 @@ logger = logging.getLogger(__name__) -def bless_throughput( +def _bless_throughput( case, test_name, baseline_root, @@ -32,6 +32,7 @@ def bless_throughput( ): success = True reason = None + below_threshold = False baseline_dir = os.path.join( baseline_root, baseline_name, case.get_value("CASEBASEID") @@ -42,20 +43,10 @@ def bless_throughput( case, baseline_dir=baseline_dir ) except FileNotFoundError as e: - success = False - comment = f"Could not read throughput file: {e!s}" except Exception as e: - success = False - comment = f"Error comparing throughput baseline: {e!s}" - # fail early - if not success: - logger.info(comment) - - return success, comment - if below_threshold: logger.info("Throughput diff appears to have been already resolved.") else: @@ -74,7 +65,7 @@ def bless_throughput( return success, reason -def bless_memory( +def _bless_memory( case, test_name, baseline_root, @@ -84,6 +75,7 @@ def bless_memory( ): success = True reason = None + below_threshold = False baseline_dir = os.path.join( baseline_root, baseline_name, case.get_value("CASEBASEID") @@ -94,20 +86,10 @@ def bless_memory( case, baseline_dir=baseline_dir ) except FileNotFoundError as e: - success = False - comment = f"Could not read memory usage file: {e!s}" except Exception as e: - success = False - comment = f"Error comparing memory baseline: {e!s}" - # fail early - if not success: - logger.info(comment) - - return success, comment - if below_threshold: logger.info("Memory usage diff appears to have been already resolved.") else: @@ -221,8 +203,6 @@ def bless_test_results( test_id=None, namelists_only=False, hist_only=False, - tput_only=False, - mem_only=False, report_only=False, force=False, pes_file=None, @@ -231,9 +211,12 @@ def bless_test_results( new_test_root=None, new_test_id=None, exclude=None, + bless_tput=False, + bless_mem=False, + bless_perf=False, **_, # Capture all for extra ): - bless_all = not (namelists_only | hist_only | tput_only | mem_only) + bless_all = not (namelists_only | hist_only) test_status_files = get_test_status_files(test_root, compiler, test_id=test_id) @@ -294,15 +277,15 @@ def bless_test_results( excluded = exclude.match(test_name) if exclude else False if (not has_no_tests and not match_test_name) or excluded: - logger.info("Skipping {!r}".format(test_name)) + logger.debug("Skipping {!r}".format(test_name)) continue overall_result, phase = ts.get_overall_test_status( ignore_namelists=True, ignore_memleak=True, - check_throughput=True, - check_memory=True, + check_throughput=False, + check_memory=False, ) # See if we need to bless namelist @@ -326,13 +309,13 @@ def bless_test_results( if hist_only or bless_all: hist_bless = bless_needed - if tput_only or bless_all: + if bless_tput or bless_perf: tput_bless = bless_needed if not tput_bless: tput_bless = ts.get_status(THROUGHPUT_PHASE) != TEST_PASS_STATUS - if mem_only or bless_all: + if bless_mem or bless_perf: mem_bless = bless_needed if not mem_bless: @@ -346,6 +329,12 @@ def bless_test_results( ) ) else: + logger.debug("Determined blesses for {!r}".format(test_name)) + logger.debug("nl_bless = {}".format(nl_bless)) + logger.debug("hist_bless = {}".format(hist_bless)) + logger.debug("tput_bless = {}".format(tput_bless)) + logger.debug("mem_bless = {}".format(mem_bless)) + logger.info( "###############################################################################" ) @@ -366,8 +355,9 @@ def bless_test_results( if baseline_name is None: baseline_name_resolved = case.get_value("BASELINE_NAME_CMP") if not baseline_name_resolved: + cime_root = CIME.utils.get_cime_root() baseline_name_resolved = CIME.utils.get_current_branch( - repo=CIME.utils.get_cime_root() + repo=cime_root ) else: baseline_name_resolved = baseline_name @@ -423,7 +413,7 @@ def bless_test_results( broken_blesses.append((test_name, reason)) if tput_bless: - success, reason = bless_throughput( + success, reason = _bless_throughput( case, test_name, baseline_root_resolved, @@ -436,7 +426,7 @@ def bless_test_results( broken_blesses.append((test_name, reason)) if mem_bless: - success, reason = bless_memory( + success, reason = _bless_memory( case, test_name, baseline_root_resolved, diff --git a/CIME/tests/test_unit_bless_test_results.py b/CIME/tests/test_unit_bless_test_results.py index 69e6de56629..fe9003d1bd2 100644 --- a/CIME/tests/test_unit_bless_test_results.py +++ b/CIME/tests/test_unit_bless_test_results.py @@ -6,8 +6,8 @@ from CIME.bless_test_results import ( bless_test_results, - bless_throughput, - bless_memory, + _bless_throughput, + _bless_memory, bless_history, bless_namelists, is_bless_needed, @@ -232,7 +232,7 @@ def test_bless_memory_force_error( case = mock.MagicMock() - success, comment = bless_memory( + success, comment = _bless_memory( case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) @@ -252,7 +252,7 @@ def test_bless_memory_force( case = mock.MagicMock() - success, comment = bless_memory( + success, comment = _bless_memory( case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) @@ -266,38 +266,44 @@ def test_bless_memory_report_only(self, perf_compare_memory_baseline): case = mock.MagicMock() - success, comment = bless_memory( + success, comment = _bless_memory( case, "SMS.f19_g16.S", "/tmp/baselines", "master", True, False ) assert success assert comment is None + @mock.patch("CIME.bless_test_results.perf_write_baseline") @mock.patch("CIME.bless_test_results.perf_compare_memory_baseline") - def test_bless_memory_general_error(self, perf_compare_memory_baseline): + def test_bless_memory_general_error( + self, perf_compare_memory_baseline, perf_write_baseline + ): perf_compare_memory_baseline.side_effect = Exception case = mock.MagicMock() - success, comment = bless_memory( - case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, False + success, comment = _bless_memory( + case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) - assert not success - assert comment == "Error comparing memory baseline: " + assert success + assert comment is None + @mock.patch("CIME.bless_test_results.perf_write_baseline") @mock.patch("CIME.bless_test_results.perf_compare_memory_baseline") - def test_bless_memory_file_not_found_error(self, perf_compare_memory_baseline): + def test_bless_memory_file_not_found_error( + self, perf_compare_memory_baseline, perf_write_baseline + ): perf_compare_memory_baseline.side_effect = FileNotFoundError case = mock.MagicMock() - success, comment = bless_memory( - case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, False + success, comment = _bless_memory( + case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) - assert not success - assert comment == "Could not read memory usage file: " + assert success + assert comment is None @mock.patch("CIME.bless_test_results.perf_compare_memory_baseline") def test_bless_memory(self, perf_compare_memory_baseline): @@ -305,7 +311,7 @@ def test_bless_memory(self, perf_compare_memory_baseline): case = mock.MagicMock() - success, comment = bless_memory( + success, comment = _bless_memory( case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, False ) @@ -322,7 +328,7 @@ def test_bless_throughput_force_error( case = mock.MagicMock() - success, comment = bless_throughput( + success, comment = _bless_throughput( case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) @@ -339,7 +345,7 @@ def test_bless_throughput_force( case = mock.MagicMock() - success, comment = bless_throughput( + success, comment = _bless_throughput( case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) @@ -353,7 +359,7 @@ def test_bless_throughput_report_only(self, perf_compare_throughput_baseline): case = mock.MagicMock() - success, comment = bless_throughput( + success, comment = _bless_throughput( case, "SMS.f19_g16.S", "/tmp/baselines", "master", True, False ) @@ -366,27 +372,30 @@ def test_bless_throughput_general_error(self, perf_compare_throughput_baseline): case = mock.MagicMock() - success, comment = bless_throughput( - case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, False + success, comment = _bless_throughput( + case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) - assert not success - assert comment == "Error comparing throughput baseline: " + assert success + assert comment is None + @mock.patch("CIME.bless_test_results.perf_write_baseline") @mock.patch("CIME.bless_test_results.perf_compare_throughput_baseline") def test_bless_throughput_file_not_found_error( - self, perf_compare_throughput_baseline + self, + perf_compare_throughput_baseline, + perf_write_baseline, ): perf_compare_throughput_baseline.side_effect = FileNotFoundError case = mock.MagicMock() - success, comment = bless_throughput( - case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, False + success, comment = _bless_throughput( + case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, True ) - assert not success - assert comment == "Could not read throughput file: " + assert success + assert comment is None @mock.patch("CIME.bless_test_results.perf_compare_throughput_baseline") def test_bless_throughput(self, perf_compare_throughput_baseline): @@ -394,13 +403,55 @@ def test_bless_throughput(self, perf_compare_throughput_baseline): case = mock.MagicMock() - success, comment = bless_throughput( + success, comment = _bless_throughput( case, "SMS.f19_g16.S", "/tmp/baselines", "master", False, False ) assert success - @mock.patch("CIME.bless_test_results.bless_memory") + @mock.patch("CIME.bless_test_results._bless_throughput") + @mock.patch("CIME.bless_test_results._bless_memory") + @mock.patch("CIME.bless_test_results.Case") + @mock.patch("CIME.bless_test_results.TestStatus") + @mock.patch("CIME.bless_test_results.get_test_status_files") + def test_bless_perf( + self, + get_test_status_files, + TestStatus, + Case, + _bless_memory, + _bless_throughput, + ): + get_test_status_files.return_value = [ + "/tmp/cases/SMS.f19_g16.S.docker_gnu/TestStatus", + ] + + ts = TestStatus.return_value + ts.get_name.return_value = "SMS.f19_g16.S.docker_gnu" + ts.get_overall_test_status.return_value = ("PASS", "RUN") + ts.get_status.side_effect = ["PASS", "PASS", "PASS", "FAIL", "FAIL"] + + case = Case.return_value.__enter__.return_value + + _bless_memory.return_value = (True, "") + + _bless_throughput.return_value = (True, "") + + success = bless_test_results( + "master", + "/tmp/baseline", + "/tmp/cases", + "gnu", + force=True, + bless_perf=True, + ) + + assert success + _bless_memory.assert_called() + _bless_throughput.assert_called() + + @mock.patch("CIME.bless_test_results._bless_throughput") + @mock.patch("CIME.bless_test_results._bless_memory") @mock.patch("CIME.bless_test_results.Case") @mock.patch("CIME.bless_test_results.TestStatus") @mock.patch("CIME.bless_test_results.get_test_status_files") @@ -409,7 +460,8 @@ def test_bless_memory_only( get_test_status_files, TestStatus, Case, - bless_memory, + _bless_memory, + _bless_throughput, ): get_test_status_files.return_value = [ "/tmp/cases/SMS.f19_g16.S.docker_gnu/TestStatus", @@ -418,11 +470,11 @@ def test_bless_memory_only( ts = TestStatus.return_value ts.get_name.return_value = "SMS.f19_g16.S.docker_gnu" ts.get_overall_test_status.return_value = ("PASS", "RUN") - ts.get_status.side_effect = ["PASS", "FAIL"] + ts.get_status.side_effect = ["PASS", "PASS", "PASS", "FAIL"] case = Case.return_value.__enter__.return_value - bless_memory.return_value = (True, "") + _bless_memory.return_value = (True, "") success = bless_test_results( "master", @@ -430,13 +482,15 @@ def test_bless_memory_only( "/tmp/cases", "gnu", force=True, - mem_only=True, + bless_mem=True, ) assert success - bless_memory.assert_called() + _bless_memory.assert_called() + _bless_throughput.assert_not_called() - @mock.patch("CIME.bless_test_results.bless_throughput") + @mock.patch("CIME.bless_test_results._bless_throughput") + @mock.patch("CIME.bless_test_results._bless_memory") @mock.patch("CIME.bless_test_results.Case") @mock.patch("CIME.bless_test_results.TestStatus") @mock.patch("CIME.bless_test_results.get_test_status_files") @@ -445,7 +499,8 @@ def test_bless_throughput_only( get_test_status_files, TestStatus, Case, - bless_throughput, + _bless_memory, + _bless_throughput, ): get_test_status_files.return_value = [ "/tmp/cases/SMS.f19_g16.S.docker_gnu/TestStatus", @@ -454,11 +509,11 @@ def test_bless_throughput_only( ts = TestStatus.return_value ts.get_name.return_value = "SMS.f19_g16.S.docker_gnu" ts.get_overall_test_status.return_value = ("PASS", "RUN") - ts.get_status.side_effect = ["PASS", "FAIL"] + ts.get_status.side_effect = ["PASS", "PASS", "PASS", "FAIL"] case = Case.return_value.__enter__.return_value - bless_throughput.return_value = (True, "") + _bless_throughput.return_value = (True, "") success = bless_test_results( "master", @@ -466,11 +521,12 @@ def test_bless_throughput_only( "/tmp/cases", "gnu", force=True, - tput_only=True, + bless_tput=True, ) assert success - bless_throughput.assert_called() + _bless_memory.assert_not_called() + _bless_throughput.assert_called() @mock.patch("CIME.bless_test_results.bless_namelists") @mock.patch("CIME.bless_test_results.Case") @@ -571,8 +627,8 @@ def test_specific(self, get_test_status_files, TestStatus, Case): assert success - @mock.patch("CIME.bless_test_results.bless_memory") - @mock.patch("CIME.bless_test_results.bless_throughput") + @mock.patch("CIME.bless_test_results._bless_memory") + @mock.patch("CIME.bless_test_results._bless_throughput") @mock.patch("CIME.bless_test_results.bless_history") @mock.patch("CIME.bless_test_results.bless_namelists") @mock.patch("CIME.bless_test_results.Case") @@ -585,12 +641,12 @@ def test_bless_tests_results_homme( Case, bless_namelists, bless_history, - bless_throughput, - bless_memory, + _bless_throughput, + _bless_memory, ): - bless_memory.return_value = (False, "") + _bless_memory.return_value = (False, "") - bless_throughput.return_value = (False, "") + _bless_throughput.return_value = (False, "") bless_history.return_value = (False, "") @@ -619,8 +675,8 @@ def test_bless_tests_results_homme( assert not success - @mock.patch("CIME.bless_test_results.bless_memory") - @mock.patch("CIME.bless_test_results.bless_throughput") + @mock.patch("CIME.bless_test_results._bless_memory") + @mock.patch("CIME.bless_test_results._bless_throughput") @mock.patch("CIME.bless_test_results.bless_history") @mock.patch("CIME.bless_test_results.bless_namelists") @mock.patch("CIME.bless_test_results.Case") @@ -633,12 +689,12 @@ def test_bless_tests_results_fail( Case, bless_namelists, bless_history, - bless_throughput, - bless_memory, + _bless_throughput, + _bless_memory, ): - bless_memory.return_value = (False, "") + _bless_memory.return_value = (False, "") - bless_throughput.return_value = (False, "") + _bless_throughput.return_value = (False, "") bless_history.return_value = (False, "") @@ -667,8 +723,8 @@ def test_bless_tests_results_fail( assert not success - @mock.patch("CIME.bless_test_results.bless_memory") - @mock.patch("CIME.bless_test_results.bless_throughput") + @mock.patch("CIME.bless_test_results._bless_memory") + @mock.patch("CIME.bless_test_results._bless_throughput") @mock.patch("CIME.bless_test_results.bless_history") @mock.patch("CIME.bless_test_results.bless_namelists") @mock.patch("CIME.bless_test_results.Case") @@ -681,12 +737,12 @@ def test_no_skip_pass( Case, bless_namelists, bless_history, - bless_throughput, - bless_memory, + _bless_throughput, + _bless_memory, ): - bless_memory.return_value = (True, "") + _bless_memory.return_value = (True, "") - bless_throughput.return_value = (True, "") + _bless_throughput.return_value = (True, "") bless_history.return_value = (True, "")