Skip to content

Commit 83454c9

Browse files
sbryngelsonclaude
andcommitted
Fix CI script issues from review: argument validation, error messages, hidden env dependency
- run-tests-with-retry.sh: extract --test-all from "$@" instead of relying on $TEST_ALL env var for retry path - check_case_optimization_output.py: restore argument validation, add per-file NaN/Inf diagnostic reporting - run_case_optimization.sh: check venv exists before loop, fix misleading error message, normalize exit code - pack.py: fix typo in Pack class comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 382c1c9 commit 83454c9

File tree

4 files changed

+34
-7
lines changed

4 files changed

+34
-7
lines changed

.github/scripts/check_case_optimization_output.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@
22

33
"""Validate case-optimization output: check D/*.dat for NaN/Inf via the packer."""
44

5+
import math
56
import sys
67
import os
78

8-
# Allow importing from the repo root (toolchain.mfc.packer)
9+
if len(sys.argv) != 2:
10+
print(f"Usage: {sys.argv[0]} <case_directory>", file=sys.stderr)
11+
sys.exit(1)
12+
13+
# Allow importing from the repo root
914
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', '..'))
1015

1116
from toolchain.mfc.packer.pack import compile as pack_compile
@@ -24,7 +29,13 @@
2429
sys.exit(1)
2530

2631
if pack.has_bad_values():
27-
print("ERROR: NaN or Inf detected in output")
32+
print("ERROR: NaN or Inf detected in output:")
33+
for name, entry in pack.entries.items():
34+
for i, val in enumerate(entry.doubles):
35+
if math.isnan(val) or math.isinf(val):
36+
label = 'NaN' if math.isnan(val) else 'Inf'
37+
print(f" {label} at index {i} in {name}")
38+
break
2839
sys.exit(1)
2940

3041
total = sum(len(e.doubles) for e in pack.entries.values())

.github/scripts/run-tests-with-retry.sh

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@
33
# of sporadic failures (up to 5). Exits non-zero on real failures.
44
# Usage: bash .github/scripts/run-tests-with-retry.sh [mfc test args...]
55

6+
# Extract flags that should carry over to retries (retries build their own
7+
# argument list with --only, so we capture passthrough flags here).
8+
PASSTHROUGH=""
9+
for arg in "$@"; do
10+
case "$arg" in
11+
--test-all) PASSTHROUGH="$PASSTHROUGH --test-all" ;;
12+
esac
13+
done
14+
615
rm -f tests/failed_uuids.txt
716
TEST_EXIT=0
817
/bin/bash mfc.sh test "$@" || TEST_EXIT=$?
@@ -15,7 +24,7 @@ if [ -s tests/failed_uuids.txt ]; then
1524
echo ""
1625
echo "=== Retrying $NUM_FAILED failed test(s): $FAILED ==="
1726
echo ""
18-
/bin/bash mfc.sh test -v --max-attempts 3 -j "$(nproc)" --only $FAILED $TEST_ALL || exit $?
27+
/bin/bash mfc.sh test -v --max-attempts 3 -j "$(nproc)" --only $FAILED $PASSTHROUGH || exit $?
1928
else
2029
echo "Too many failures ($NUM_FAILED) to retry — likely a real issue."
2130
exit 1

.github/scripts/run_case_optimization.sh

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ if [ "$job_device" = "gpu" ] && [ "$ngpus" -eq 0 ]; then
1313
ngpus=1
1414
fi
1515

16+
# Verify the venv Python interpreter exists (created by ./mfc.sh build)
17+
if [ ! -x build/venv/bin/python3 ]; then
18+
echo "ERROR: build/venv/bin/python3 not found."
19+
echo "The MFC build venv may not have been created. Was the pre-build step successful?"
20+
exit 1
21+
fi
22+
1623
benchmarks=(
1724
benchmarks/5eq_rk3_weno3_hllc/case.py
1825
benchmarks/viscous_weno5_sgb_acoustic/case.py
@@ -43,7 +50,7 @@ for case in "${benchmarks[@]}"; do
4350
echo "PASS: $case_name"
4451
passed=$((passed + 1))
4552
else
46-
echo "FAIL: $case_name (NaN/Inf in output)"
53+
echo "FAIL: $case_name (validation error)"
4754
failed=$((failed + 1))
4855
failed_cases="$failed_cases $case_name"
4956
fi
@@ -65,4 +72,4 @@ if [ $failed -gt 0 ]; then
6572
fi
6673
echo "========================================"
6774

68-
exit $failed
75+
[ $failed -eq 0 ] && exit 0 || exit 1

toolchain/mfc/packer/pack.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ def __repr__(self) -> str:
2020
return f"{self.filepath} {' '.join([ str(d) for d in self.doubles ])}"
2121

2222

23-
# This class maps to the data contained in the entirety of D/: it is tush a list
24-
# of PackEntry classes.
23+
# This class maps to the data contained in the entirety of D/: a dictionary
24+
# of PackEntry instances keyed by filepath.
2525
class Pack:
2626
entries: typing.Dict[str, PackEntry]
2727

0 commit comments

Comments
 (0)