From 04ae726784bd114951538dc601b706c9e09138ec Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 11:51:13 -0400 Subject: [PATCH 1/7] better hashes --- toolchain/mfc/build.py | 84 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 9 deletions(-) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 7c7648ae18..6bb5a5bcfd 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -1,4 +1,8 @@ -import os, typing, hashlib, dataclasses +import os, typing, dataclasses +import re +import pathlib +import subprocess +import tempfile from .case import Case from .printer import cons @@ -32,19 +36,81 @@ def compute(self) -> typing.Set: def __hash__(self) -> int: return hash(self.name) + def get_compiler_info(self) -> str: + with tempfile.TemporaryDirectory() as build_dir: + subprocess.run( + ["cmake", "-B", build_dir, "-S", ".", "-DCMAKE_VERBOSE_MAKEFILE=ON"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + + cache = pathlib.Path(build_dir) / "CMakeCache.txt" + if not cache.exists(): + return "unknown" + + pat = re.compile(r"CMAKE_.*_COMPILER:FILEPATH=(.+)", re.IGNORECASE) + compiler_name = "" + + for line in cache.read_text(errors="ignore").splitlines(): + if "fortran" in line.lower(): + m = pat.search(line) + if m: + compiler_name = pathlib.Path(m.group(1).strip()).name + break + + name = compiler_name.lower() + if "gfortran" in name: + return "gnu" + if "ifort" in name or "ifx" in name: + return "intel" + if "nvfortran" in name: + return "nvhpc" + if name == "ftn" or "cray" in name: + return "cray" + if "pgfortran" in name: + return "pgi" + if "clang" in name: + return "clang" + if "flang" in name: + return "flang" + return "unknown" + def get_slug(self, case: Case ) -> str: if self.isDependency: return self.name - m = hashlib.sha256() - m.update(self.name.encode()) - m.update(CFG().make_slug().encode()) - m.update(case.get_fpp(self, False).encode()) - + # Start with target name + parts = [self.name] + + # Add active configuration options + cfg = CFG() + cfg_parts = [] + for key, value in sorted(cfg.items()): + if value: # Only include enabled options + cfg_parts.append(key) + + if cfg_parts: + parts.append('-'.join(cfg_parts)) + + # Add chemistry info if enabled if case.params.get('chemistry', 'F') == 'T': - m.update(case.get_cantera_solution().name.encode()) - - return m.hexdigest()[:10] + parts.append(f"chem-{case.get_cantera_solution().name}") + + # Add case optimization if enabled + if case.params.get('case_optimization', False): + parts.append('opt') + + # Add compiler identifier + try: + compiler_id = self.get_compiler_info() + if compiler_id and compiler_id != 'unknown': + parts.append(f"fc-{compiler_id}") + except: + parts.append(f"fc-unknown") + + # Join all parts with underscores + return '_'.join(parts) # Get path to directory that will store the build files def get_staging_dirpath(self, case: Case ) -> str: From 495282626c20671eb131cabfcb454184e00b67a0 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 12:05:39 -0400 Subject: [PATCH 2/7] fix lint --- toolchain/mfc/build.py | 96 +++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 6bb5a5bcfd..f5219ad111 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -37,44 +37,45 @@ def __hash__(self) -> int: return hash(self.name) def get_compiler_info(self) -> str: - with tempfile.TemporaryDirectory() as build_dir: - subprocess.run( - ["cmake", "-B", build_dir, "-S", ".", "-DCMAKE_VERBOSE_MAKEFILE=ON"], - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - ) - - cache = pathlib.Path(build_dir) / "CMakeCache.txt" - if not cache.exists(): - return "unknown" - - pat = re.compile(r"CMAKE_.*_COMPILER:FILEPATH=(.+)", re.IGNORECASE) - compiler_name = "" - - for line in cache.read_text(errors="ignore").splitlines(): - if "fortran" in line.lower(): - m = pat.search(line) - if m: - compiler_name = pathlib.Path(m.group(1).strip()).name - break - - name = compiler_name.lower() - if "gfortran" in name: - return "gnu" - if "ifort" in name or "ifx" in name: - return "intel" - if "nvfortran" in name: - return "nvhpc" - if name == "ftn" or "cray" in name: - return "cray" - if "pgfortran" in name: - return "pgi" - if "clang" in name: - return "clang" - if "flang" in name: - return "flang" - return "unknown" + compiler = "unknown" + with tempfile.TemporaryDirectory() as build_dir: + subprocess.run( + ["cmake", "-B", build_dir, "-S", ".", "-DCMAKE_VERBOSE_MAKEFILE=ON"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + + cache = pathlib.Path(build_dir) / "CMakeCache.txt" + if not cache.exists(): + compiler = "unknown" + + pat = re.compile(r"CMAKE_.*_COMPILER:FILEPATH=(.+)", re.IGNORECASE) + compiler_name = "" + + for line in cache.read_text(errors="ignore").splitlines(): + if "fortran" in line.lower(): + m = pat.search(line) + if m: + compiler_name = pathlib.Path(m.group(1).strip()).name + break + + name = compiler_name.lower() + if "gfortran" in name: + compiler = "gnu" + if "ifort" in name or "ifx" in name: + compiler = "intel" + if "nvfortran" in name: + compiler = "nvhpc" + if name == "ftn" or "cray" in name: + compiler = "cray" + if "pgfortran" in name: + compiler = "pgi" + if "clang" in name: + compiler = "clang" + if "flang" in name: + compiler = "flang" + return compiler def get_slug(self, case: Case ) -> str: if self.isDependency: @@ -82,32 +83,29 @@ def get_slug(self, case: Case ) -> str: # Start with target name parts = [self.name] - + # Add active configuration options cfg = CFG() cfg_parts = [] for key, value in sorted(cfg.items()): if value: # Only include enabled options cfg_parts.append(key) - + if cfg_parts: parts.append('-'.join(cfg_parts)) - + # Add chemistry info if enabled if case.params.get('chemistry', 'F') == 'T': parts.append(f"chem-{case.get_cantera_solution().name}") - + # Add case optimization if enabled if case.params.get('case_optimization', False): parts.append('opt') - + # Add compiler identifier - try: - compiler_id = self.get_compiler_info() - if compiler_id and compiler_id != 'unknown': - parts.append(f"fc-{compiler_id}") - except: - parts.append(f"fc-unknown") + compiler_id = self.get_compiler_info() + if compiler_id: + parts.append(f"fc-{compiler_id}") # Join all parts with underscores return '_'.join(parts) From 524fd70fd42c1a3a32bbd8eaba17a232ea017026 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 12:13:15 -0400 Subject: [PATCH 3/7] Update toolchain/mfc/build.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- toolchain/mfc/build.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index f5219ad111..6b049e04ff 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -39,12 +39,15 @@ def __hash__(self) -> int: def get_compiler_info(self) -> str: compiler = "unknown" with tempfile.TemporaryDirectory() as build_dir: - subprocess.run( - ["cmake", "-B", build_dir, "-S", ".", "-DCMAKE_VERBOSE_MAKEFILE=ON"], - check=True, - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - ) + try: + subprocess.run( + ["cmake", "-B", build_dir, "-S", ".", "-DCMAKE_VERBOSE_MAKEFILE=ON"], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + except (subprocess.CalledProcessError, FileNotFoundError): + return "unknown" cache = pathlib.Path(build_dir) / "CMakeCache.txt" if not cache.exists(): From 004dd1e3bebe5c31c145085f41e1278fea65d3bc Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 12:15:34 -0400 Subject: [PATCH 4/7] Update toolchain/mfc/build.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- toolchain/mfc/build.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 6b049e04ff..23001c664a 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -66,17 +66,17 @@ def get_compiler_info(self) -> str: name = compiler_name.lower() if "gfortran" in name: compiler = "gnu" - if "ifort" in name or "ifx" in name: + elif "ifort" in name or "ifx" in name: compiler = "intel" - if "nvfortran" in name: + elif "nvfortran" in name: compiler = "nvhpc" - if name == "ftn" or "cray" in name: + elif name == "ftn" or "cray" in name: compiler = "cray" - if "pgfortran" in name: + elif "pgfortran" in name: compiler = "pgi" - if "clang" in name: + elif "clang" in name: compiler = "clang" - if "flang" in name: + elif "flang" in name: compiler = "flang" return compiler From d42a0f739e5131e7f56a7ecbdb7fe22fc17ae46d Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 12:17:08 -0400 Subject: [PATCH 5/7] Update toolchain/mfc/build.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- toolchain/mfc/build.py | 1 + 1 file changed, 1 insertion(+) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 23001c664a..135e161c46 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -52,6 +52,7 @@ def get_compiler_info(self) -> str: cache = pathlib.Path(build_dir) / "CMakeCache.txt" if not cache.exists(): compiler = "unknown" + return compiler pat = re.compile(r"CMAKE_.*_COMPILER:FILEPATH=(.+)", re.IGNORECASE) compiler_name = "" From a7e7d198d14dabfa75f58459f8f666eb34895f49 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 12:24:37 -0400 Subject: [PATCH 6/7] Update toolchain/mfc/build.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- toolchain/mfc/build.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/toolchain/mfc/build.py b/toolchain/mfc/build.py index 135e161c46..c9d63301ca 100644 --- a/toolchain/mfc/build.py +++ b/toolchain/mfc/build.py @@ -51,8 +51,7 @@ def get_compiler_info(self) -> str: cache = pathlib.Path(build_dir) / "CMakeCache.txt" if not cache.exists(): - compiler = "unknown" - return compiler + return "unknown" pat = re.compile(r"CMAKE_.*_COMPILER:FILEPATH=(.+)", re.IGNORECASE) compiler_name = "" From 4a57558ff3fc25bccf6677f5c95b0be9086d7bf9 Mon Sep 17 00:00:00 2001 From: Spencer Bryngelson Date: Sun, 10 Aug 2025 12:27:50 -0400 Subject: [PATCH 7/7] Update .pr_agent.toml --- .pr_agent.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.pr_agent.toml b/.pr_agent.toml index af24b7074a..adc87d0a0c 100644 --- a/.pr_agent.toml +++ b/.pr_agent.toml @@ -4,12 +4,12 @@ pr_commands = ["/describe", "/review", "/improve"] [pr_reviewer] # (all fields optional) -num_max_findings = 5 # how many items to surface +num_max_findings = 6 # how many items to surface require_tests_review = true extra_instructions = """ -Focus on duplicate code, the possibility of bugs, and if the PR added appropriate tests if it added a simulation feature. +Focus on duplicate code, the possibility of bugs, and whether the PR added appropriate tests if it added a simulation feature. """ [pr_code_suggestions] -commitable_code_suggestions = false # purely advisory, no write ops +commitable_code_suggestions = true # purely advisory, no write ops apply_suggestions_checkbox = false # hides the “Apply/Chat” boxes