Improve image checker (#699)

* Improve image checker

* Clean up code

Author: forsyth2

.gitignore

@@ -6,6 +6,7 @@ zppy.egg-info/
 \#*
 
 examples/example_compy.cfg
+min_case_summary.md
 test_*_output
 tests/*.cfg.txt
 tests/integration/image_check_failures*

tests/integration/test_images.py

@@ -0,0 +1,192 @@
+import os
+from math import ceil
+from typing import Dict
+
+import matplotlib.backends.backend_pdf
+import matplotlib.image as mpimg
+from mache import MachineInfo
+from matplotlib import pyplot as plt
+
+from tests.integration.utils import Results, check_mismatched_images, get_expansions
+
+V3_CASE_NAME = "v3.LR.historical_0051"
+V2_CASE_NAME = "v2.LR.historical_0201"
+
+
+# TODO: fix issue where blank plots generate after so many pages in the PDF
+def make_image_diff_grid(diff_subdir, pdf_name="image_diff_grid.pdf", rows_per_page=2):
+    machine_info = MachineInfo()
+    web_portal_base_path = machine_info.config.get("web_portal", "base_path")
+    web_portal_base_url = machine_info.config.get("web_portal", "base_url")
+    print(f"web_portal_base_path: {web_portal_base_path}")
+    print(f"web_portal_base_url: {web_portal_base_url}")
+    print(f"Making image diff grid for {diff_subdir}")
+
+    if not diff_subdir.startswith(web_portal_base_path):
+        print(
+            f"diff_subdir {diff_subdir} is not a subdir of web_portal_base_path: {web_portal_base_path}"
+        )
+        return
+    pdf_path = f"{diff_subdir}/{pdf_name}"
+    pdf = matplotlib.backends.backend_pdf.PdfPages(pdf_path)
+    print(f"Saving to:\n{pdf_path}")
+    web_subdir = diff_subdir.removeprefix(web_portal_base_path)
+    print(f"Web page will be at:\n{web_portal_base_url}/{web_subdir}/{pdf_name}")
+
+    prefixes = []
+    # print(f"Walking diff_subdir: {diff_subdir}")
+    for root, _, files in os.walk(diff_subdir):
+        # print(f"root: {root}")
+        for file_name in files:
+            # print(f"file_name: {file_name}")
+            if file_name.endswith("_diff.png"):
+                prefixes.append(f"{root}/{file_name.split('_diff.png')[0]}")
+    rows = len(prefixes)
+    if rows == 0:
+        # No diffs to collect into a PDF
+        return
+    cols = 3  # actual, expected, diff
+    print(f"Constructing a {rows}x{cols} grid of image diffs")
+
+    num_pages = ceil(rows / rows_per_page)
+    for page in range(num_pages):
+        fig, axes = plt.subplots(rows_per_page, cols)
+        print(f"Page {page}")
+        for i, ax_row in enumerate(axes):
+            count = page * 3 + i
+            if count > len(prefixes) - 1:
+                break
+            # We already know all the files are in `diff_subdir`; no need to repeat it.
+            short_title = prefixes[count].removeprefix(diff_subdir)
+            print(f"short_title {i}: {short_title}")
+            ax_row[1].set_title(short_title, fontsize=6)
+            img = mpimg.imread(f"{prefixes[count]}_actual.png")
+            ax_row[0].imshow(img)
+            ax_row[0].set_xticks([])
+            ax_row[0].set_yticks([])
+            img = mpimg.imread(f"{prefixes[count]}_expected.png")
+            ax_row[1].imshow(img)
+            ax_row[1].set_xticks([])
+            ax_row[1].set_yticks([])
+            img = mpimg.imread(f"{prefixes[count]}_diff.png")
+            ax_row[2].imshow(img)
+            ax_row[2].set_xticks([])
+            ax_row[2].set_yticks([])
+        fig.tight_layout()
+        pdf.savefig(1)
+        plt.close(fig)
+    pdf.close()
+    plt.close("all")
+    print(f"Reminder:\n{web_portal_base_url}/{web_subdir}/{pdf_name}")
+
+
+def check_images(expansions, cfg_specifier, case_name, task, diff_dir_suffix=""):
+    expected_dir = expansions["expected_dir"]
+    user_www = expansions["user_www"]
+    unique_id = expansions["unique_id"]
+    actual_images_dir = (
+        f"{user_www}zppy_weekly_{cfg_specifier}_www/{unique_id}/{case_name}/"
+    )
+
+    # The expected_images_file lists all images we expect to compare.
+    expected_images_file = f"{expected_dir}image_list_expected_{cfg_specifier}.txt"
+    expected_images_dir = f"{expected_dir}expected_{cfg_specifier}"
+
+    # The directory to place differences in.
+    diff_dir = (
+        f"{actual_images_dir}image_check_failures_{cfg_specifier}{diff_dir_suffix}"
+    )
+
+    test_results = check_mismatched_images(
+        actual_images_dir,
+        expected_images_file,
+        expected_images_dir,
+        diff_dir,
+        task,
+    )
+    diff_subdir = f"{diff_dir}/{task}"
+    # Write missing and mismatched images to files
+    missing_images_file = f"{diff_subdir}/missing_images.txt"
+    if os.path.exists(missing_images_file):
+        os.remove(missing_images_file)
+    for missing_image in test_results.file_list_missing:
+        with open(missing_images_file, "a") as f:
+            f.write(f"{missing_image}\n")
+    mismatched_images_file = f"{diff_subdir}/mismatched_images.txt"
+    if os.path.exists(mismatched_images_file):
+        os.remove(mismatched_images_file)
+    for mismatched_image in test_results.file_list_mismatched:
+        with open(mismatched_images_file, "a") as f:
+            f.write(f"{mismatched_image}\n")
+    # Create image diff grid
+    # make_image_diff_grid(diff_subdir)
+    return test_results
+
+
+def construct_markdown_summary_table(
+    test_results_dict: Dict[str, Results], output_file_path: str
+):
+    with open(output_file_path, "w") as f:
+        f.write("# Summary of test results\n\n")
+        f.write(
+            "Diff subdir is where to find the lists of missing/mismatched images, the image diff grid, and the individual diffs.\n"
+        )
+        f.write("Note image diff grids can not yet be constructed automatically.\n")
+        f.write(
+            "| Test name | Total images | Correct images | Missing images | Mismatched images | Diff subdir | \n"
+        )
+        f.write("| --- | --- | --- | --- | --- | --- | \n")
+        for test_name, test_results in test_results_dict.items():
+            f.write(
+                f"| {test_name} | {test_results.image_count_total} | {test_results.image_count_correct} | {test_results.image_count_missing} | {test_results.image_count_mismatched} | {test_results.diff_dir}/{test_results.task} | \n"
+            )
+
+
+def test_images():
+    # To test a different branch, set this to True, and manually set the expansions.
+    TEST_DIFFERENT_EXPANSIONS = False
+    if TEST_DIFFERENT_EXPANSIONS:
+        expansions = dict()
+        # Example settings:
+        expansions["expected_dir"] = "/lcrc/group/e3sm/public_html/zppy_test_resources/"
+        expansions["user_www"] = (
+            "/lcrc/group/e3sm/public_html/diagnostic_output/ac.forsyth2/"
+        )
+        expansions["unique_id"] = "test_zppy_20250401"
+        diff_dir_suffix = "_test_pr699_try6"
+    else:
+        expansions = get_expansions()
+        diff_dir_suffix = ""
+    test_results_dict: Dict[str, Results] = dict()
+    for task in ["e3sm_diags", "mpas_analysis", "global_time_series", "ilamb"]:
+        test_results = check_images(
+            expansions,
+            "comprehensive_v2",
+            V2_CASE_NAME,
+            task,
+            diff_dir_suffix=diff_dir_suffix,
+        )
+        test_results_dict[f"comprehensive_v2_{task}"] = test_results
+    for task in ["e3sm_diags", "mpas_analysis", "global_time_series", "ilamb"]:
+        test_results = check_images(
+            expansions,
+            "comprehensive_v3",
+            V3_CASE_NAME,
+            task,
+            diff_dir_suffix=diff_dir_suffix,
+        )
+        test_results_dict[f"comprehensive_v3_{task}"] = test_results
+    for task in ["e3sm_diags", "global_time_series", "ilamb"]:  # No mpas_analysis
+        test_results = check_images(
+            expansions,
+            "bundles",
+            V3_CASE_NAME,
+            task,
+            diff_dir_suffix=diff_dir_suffix,
+        )
+        test_results_dict[f"bundles_{task}"] = test_results
+    md_path = "min_case_summary.md"
+    construct_markdown_summary_table(test_results_dict, md_path)
+    print(f"Copy output of {md_path} to a PR comment.")
+    for tr in test_results_dict.values():
+        assert tr.image_count_total == tr.image_count_correct

tests/integration/test_weekly.py

@@ -28,7 +28,7 @@ def check_images(test_name, case_name, subdir):
         expected_images_file,
         expected_images_dir,
         diff_dir,
-        [subdir],
+        subdir,
     )
 
 

tests/integration/utils.py

@@ -12,6 +12,31 @@
 # Image checking ##########################################################
 
 
+# Originally in https://github.com/E3SM-Project/zppy/pull/695
+# And https://github.com/E3SM-Project/zppy/pull/698
+class Results(object):
+    def __init__(
+        self,
+        diff_dir: str,
+        task: str,
+        image_count_total: int,
+        file_list_missing: List[str],
+        file_list_mismatched: List[str],
+    ):
+        if image_count_total == 0:
+            raise ValueError(f"No images found for task {task} in {diff_dir}")
+        self.diff_dir = diff_dir
+        self.task = task
+        self.image_count_total = image_count_total
+        self.image_count_missing = len(file_list_missing)
+        self.image_count_mismatched = len(file_list_mismatched)
+        self.image_count_correct = (
+            image_count_total - len(file_list_missing) - len(file_list_mismatched)
+        )
+        self.file_list_missing = sorted(file_list_missing)
+        self.file_list_mismatched = sorted(file_list_mismatched)
+
+
 # Copied from E3SM Diags
 def compare_images(
     missing_images,
@@ -92,24 +117,24 @@ def compare_images(
 
 
 def check_mismatched_images(
-    actual_images_dir,
-    expected_images_file,
-    expected_images_dir,
-    diff_dir,
-    subdirs_to_check,
-):
+    actual_images_dir: str,
+    expected_images_file: str,
+    expected_images_dir: str,
+    diff_dir: str,
+    task: str,
+) -> Results:
     missing_images: List[str] = []
     mismatched_images: List[str] = []
 
     counter = 0
+    print(f"Opening expected images file {expected_images_file}")
     with open(expected_images_file) as f:
+        print(f"Reading expected images file {expected_images_file}")
         for line in f:
             image_name = line.strip("./").strip("\n")
             proceed = False
-            for subdir in subdirs_to_check:
-                if image_name.startswith(subdir):
-                    proceed = True
-                    break
+            if image_name.startswith(task):
+                proceed = True
             if proceed:
                 counter += 1
                 if counter % 250 == 0:
@@ -142,6 +167,7 @@ def check_mismatched_images(
     print(
         f"Number of correct images: {counter - len(missing_images) - len(mismatched_images)}"
     )
+    test_results = Results(diff_dir, task, counter, missing_images, mismatched_images)
 
     # Make diff_dir readable
     if os.path.exists(diff_dir):
@@ -151,8 +177,7 @@ def check_mismatched_images(
         # That is, if we're in this case, we expect the following:
         assert len(missing_images) == counter
 
-    assert missing_images == []
-    assert mismatched_images == []
+    return test_results
 
 
 # Multi-machine testing ##########################################################