Skip to content

Commit

Permalink
Merge pull request #4506 from ESMCI/another_bless_test_results_fix
Browse files Browse the repository at this point in the history 
More bless_test_results fixes

Adds testing for bless_test_results function
Fixes --pes-file argument variable name
Fixes duplicate -g for create_test when blessing namelists
Fixes checking if memory/throughput phase failed
Fixes variable names causing blessing memory/throughput to be skipped
Test suite:
Test baseline:
Test namelist changes:
Test status: [bit for bit, roundoff, climate changing]

Fixes [CIME Github issue #]
User interface changes?:
Update gh-pages html (Y/N)?:
  • Loading branch information
@jgfouca
jgfouca authored Oct 25, 2023
2 parents e47e4c1 + 89fa052 commit 7b57911
Show file tree
Hide file tree
Showing 5 changed files with 1,068 additions and 71 deletions.
71 changes: 45 additions & 26 deletions CIME/SystemTests/system_tests_common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,40 +686,59 @@ def _compare_memory(self):
Compares current test memory usage to baseline.
"""
with self._test_status:
below_tolerance, comment = perf_compare_memory_baseline(self._case)

if below_tolerance is not None:
append_testlog(comment, self._orig_caseroot)
try:
below_tolerance, comment = perf_compare_memory_baseline(self._case)
except Exception as e:
logger.info("Failed to compare memory usage baseline: {!s}".format(e))

if (
below_tolerance
and self._test_status.get_status(MEMCOMP_PHASE) is None
):
self._test_status.set_status(MEMCOMP_PHASE, TEST_PASS_STATUS)
elif self._test_status.get_status(MEMCOMP_PHASE) != TEST_FAIL_STATUS:
self._test_status.set_status(
MEMCOMP_PHASE, TEST_FAIL_STATUS, comments=comment
)
self._test_status.set_status(
MEMCOMP_PHASE, TEST_FAIL_STATUS, comments=str(e)
)
else:
if below_tolerance is not None:
append_testlog(comment, self._orig_caseroot)

if (
below_tolerance
and self._test_status.get_status(MEMCOMP_PHASE) is None
):
self._test_status.set_status(MEMCOMP_PHASE, TEST_PASS_STATUS)
elif (
self._test_status.get_status(MEMCOMP_PHASE) != TEST_FAIL_STATUS
):
self._test_status.set_status(
MEMCOMP_PHASE, TEST_FAIL_STATUS, comments=comment
)

def _compare_throughput(self):
"""
Compares current test throughput to baseline.
"""
with self._test_status:
below_tolerance, comment = perf_compare_throughput_baseline(self._case)

if below_tolerance is not None:
append_testlog(comment, self._orig_caseroot)
try:
below_tolerance, comment = perf_compare_throughput_baseline(self._case)
except Exception as e:
logger.info("Failed to compare throughput baseline: {!s}".format(e))

if (
below_tolerance
and self._test_status.get_status(THROUGHPUT_PHASE) is None
):
self._test_status.set_status(THROUGHPUT_PHASE, TEST_PASS_STATUS)
elif self._test_status.get_status(THROUGHPUT_PHASE) != TEST_FAIL_STATUS:
self._test_status.set_status(
THROUGHPUT_PHASE, TEST_FAIL_STATUS, comments=comment
)
self._test_status.set_status(
THROUGHPUT_PHASE, TEST_FAIL_STATUS, comments=str(e)
)
else:
if below_tolerance is not None:
append_testlog(comment, self._orig_caseroot)

if (
below_tolerance
and self._test_status.get_status(THROUGHPUT_PHASE) is None
):
self._test_status.set_status(THROUGHPUT_PHASE, TEST_PASS_STATUS)
elif (
self._test_status.get_status(THROUGHPUT_PHASE)
!= TEST_FAIL_STATUS
):
self._test_status.set_status(
THROUGHPUT_PHASE, TEST_FAIL_STATUS, comments=comment
)

def _compare_baseline(self):
"""
Expand Down
18 changes: 2 additions & 16 deletions CIME/baselines/performance.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,7 @@ def perf_compare_throughput_baseline(case, baseline_dir=None):

baseline_file = os.path.join(baseline_dir, "cpl-tput.log")

try:
baseline = read_baseline_file(baseline_file)
except FileNotFoundError as e:
comment = f"Could not read baseline throughput file: {e!s}"

logger.debug(comment)

return None, comment
baseline = read_baseline_file(baseline_file)

tolerance = case.get_value("TEST_TPUT_TOLERANCE")

Expand Down Expand Up @@ -90,14 +83,7 @@ def perf_compare_memory_baseline(case, baseline_dir=None):

baseline_file = os.path.join(baseline_dir, "cpl-mem.log")

try:
baseline = read_baseline_file(baseline_file)
except FileNotFoundError as e:
comment = f"Could not read baseline memory usage: {e!s}"

logger.debug(comment)

return None, comment
baseline = read_baseline_file(baseline_file)

tolerance = case.get_value("TEST_MEMLEAK_TOLERANCE")

Expand Down
74 changes: 55 additions & 19 deletions CIME/bless_test_results.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,27 @@ def bless_throughput(
baseline_root, baseline_name, case.get_value("CASEBASEID")
)

below_threshold, comment = perf_compare_throughput_baseline(
case, baseline_dir=baseline_dir
)
try:
below_threshold, comment = perf_compare_throughput_baseline(
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("Diff appears to have been already resolved.")
logger.info("Throughput diff appears to have been already resolved.")
else:
logger.info(comment)

Expand Down Expand Up @@ -74,12 +89,27 @@ def bless_memory(
baseline_root, baseline_name, case.get_value("CASEBASEID")
)

below_threshold, comment = perf_compare_memory_baseline(
case, baseline_dir=baseline_dir
)
try:
below_threshold, comment = perf_compare_memory_baseline(
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("Diff appears to have been already resolved.")
logger.info("Memory usage diff appears to have been already resolved.")
else:
logger.info(comment)

Expand All @@ -101,7 +131,7 @@ def bless_namelists(
test_name,
report_only,
force,
pesfile,
pes_file,
baseline_name,
baseline_root,
new_test_root=None,
Expand All @@ -119,20 +149,21 @@ def bless_namelists(
):
config = Config.instance()

create_test_gen_args = " -g {} ".format(
baseline_name
create_test_gen_args = (
" -g {} ".format(baseline_name)
if config.create_test_flag_mode == "cesm"
else " -g -b {} ".format(baseline_name)
)

if new_test_root is not None:
create_test_gen_args += " --test-root={0} --output-root={0} ".format(
new_test_root
)
if new_test_id is not None:
create_test_gen_args += " -t {}".format(new_test_id)

if pesfile is not None:
create_test_gen_args += " --pesfile {}".format(pesfile)
if pes_file is not None:
create_test_gen_args += " --pesfile {}".format(pes_file)

stat, out, _ = run_cmd(
"{}/create_test {} --namelists-only {} --baseline-root {} -o".format(
Expand All @@ -148,9 +179,7 @@ def bless_namelists(
return True, None


###############################################################################
def bless_history(test_name, case, baseline_name, baseline_root, report_only, force):
###############################################################################
real_user = case.get_value("REALUSER")
with EnvironmentContext(USER=real_user):

Expand Down Expand Up @@ -196,7 +225,7 @@ def bless_test_results(
mem_only=False,
report_only=False,
force=False,
pesfile=None,
pes_file=None,
bless_tests=None,
no_skip_pass=False,
new_test_root=None,
Expand Down Expand Up @@ -242,6 +271,7 @@ def bless_test_results(
testopts = parse_test_name(test_name)[1]
testopts = [] if testopts is None else testopts
build_only = "B" in testopts
# TODO test_name will never be None otherwise `parse_test_name` would raise an error
if test_name is None:
case_dir = os.path.basename(test_dir)
test_name = CIME.utils.normalize_case_id(case_dir)
Expand Down Expand Up @@ -299,9 +329,15 @@ def bless_test_results(
if tput_only or bless_all:
tput_bless = bless_needed

if not tput_bless:
tput_bless = ts.get_status(THROUGHPUT_PHASE) != TEST_PASS_STATUS

if mem_only or bless_all:
mem_bless = bless_needed

if not mem_bless:
mem_bless = ts.get_status(MEMCOMP_PHASE) != TEST_PASS_STATUS

# Now, do the bless
if not nl_bless and not hist_bless and not tput_bless and not mem_bless:
logger.info(
Expand Down Expand Up @@ -359,7 +395,7 @@ def bless_test_results(
test_name,
report_only,
force,
pesfile,
pes_file,
baseline_name_resolved,
baseline_root_resolved,
new_test_root=new_test_root,
Expand All @@ -386,7 +422,7 @@ def bless_test_results(
if not success:
broken_blesses.append((test_name, reason))

if tput_only:
if tput_bless:
success, reason = bless_throughput(
case,
test_name,
Expand All @@ -399,7 +435,7 @@ def bless_test_results(
if not success:
broken_blesses.append((test_name, reason))

if mem_only:
if mem_bless:
success, reason = bless_memory(
case,
test_name,
Expand Down
14 changes: 4 additions & 10 deletions CIME/tests/test_unit_baselines_performance.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,8 @@ def test_perf_compare_throughput_baseline_no_baseline_file(
0.05,
)

(below_tolerance, comment) = performance.perf_compare_throughput_baseline(
case
)

assert below_tolerance is None
assert comment == "Could not read baseline throughput file: "
with self.assertRaises(FileNotFoundError):
performance.perf_compare_throughput_baseline(case)

@mock.patch("CIME.baselines.performance._perf_get_throughput")
@mock.patch("CIME.baselines.performance.read_baseline_file")
Expand Down Expand Up @@ -526,10 +522,8 @@ def test_perf_compare_memory_baseline_no_baseline_file(
0.05,
)

(below_tolerance, comment) = performance.perf_compare_memory_baseline(case)

assert below_tolerance is None
assert comment == "Could not read baseline memory usage: "
with self.assertRaises(FileNotFoundError):
performance.perf_compare_memory_baseline(case)

@mock.patch("CIME.baselines.performance.get_cpl_mem_usage")
@mock.patch("CIME.baselines.performance.read_baseline_file")
Expand Down
Loading

0 comments on commit 7b57911

Please sign in to comment.