Skip to content

Commit e668548

Browse files
committed
Implement P1-P37: deep audit bug fixes and production hardening
Critical/High fixes: - P1: Fix VTKWriter destructor race (set stop_ under mutex, notify_all) - P2: Fix multi-GPU halo exchange cross-neighbor event synchronization - P3: Fix idx3d integer overflow (use long long intermediate) - P4: Fix Makefile CUDA coverage flags (-Xcompiler= for nvcc) - P5: Thread --saturation-threshold through RenderConfig to render_frame Medium fixes: - P6: Make --use_fast_math opt-in CMake option (default OFF) - P7: Call validate() in from_json() and from_json_string() - P8: Replace std::signal with POSIX sigaction() for thread safety - P9: Fix CheckpointIO to use POSIX write+fsync on the actual write fd - P10: Add dimension/spacing sanity checks on checkpoint read - P11: Add validation for output.format, checkpoint.keep_last, seed_radius - P13: Write raw VTK files atomically (tmp + rename) - P14: Add CRC32 checksum to checkpoint binary format - P15: Fix saturation shading to use sat_steps.max() not steps.max() - P16: Use prescan saturated[] by step lookup instead of recomputing - P17: Clean up self-test tmpdir - P18: Fix video stitching macro_block_size=2 for yuv420p - P19: Validate --window-size (positive, even) - P20: Build step-indexed lookup for prescan/frame alignment - P21: Combine ax/ax2 legend handles in time-series sidebar - P22: Replace FieldData::copy_from assert with runtime exception - P23: Add build-and-test to CI release-docker needs - P24: Add viz resume/skip for existing PNGs - P25: Fix font fallback to pass size kwarg to load_default() - P26: Add saturation detection and config validation tests - P27: Add error handling for video codec initialization failures Low-priority fixes: - P28: Use notify_all() in write_async for robustness - P29: Pre-allocate reduction scratch buffer to avoid per-call allocation - P30: Add #pragma pack(push,1) to CheckpointIO Header - P31: Warn on silently filtered discover_frames files - P33/P34: Document double I/O and --prescan-limit sidebar behavior - P35: Draw saturation badge directly on canvas (avoid copy) - P36: Fix render_slice_panel fallback alpha to 255 - P37: Print deprecation warning when --view is used https://claude.ai/code/session_01X3j3NGmRJ7pCS63FPkPwP8
1 parent bf0cba0 commit e668548

16 files changed

Lines changed: 428 additions & 113 deletions

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ jobs:
222222
release-docker:
223223
name: Push Docker Images
224224
runs-on: ubuntu-latest
225-
needs: [build-cpu-only, docker-build, format-check, k8s-validate]
225+
needs: [build-and-test, build-cpu-only, docker-build, format-check, k8s-validate]
226226
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
227227
permissions:
228228
packages: write

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ test-coverage: ## Build with coverage instrumentation and generate report
160160
@echo "==> Configuring with coverage flags..."
161161
cmake --preset $(CMAKE_PRESET_DEBUG) -G $(CMAKE_GENERATOR) \
162162
-DCMAKE_CXX_FLAGS="--coverage -fprofile-arcs -ftest-coverage" \
163-
-DCMAKE_CUDA_FLAGS="--coverage -fprofile-arcs -ftest-coverage" \
163+
-DCMAKE_CUDA_FLAGS="-Xcompiler=--coverage,-fprofile-arcs,-ftest-coverage" \
164164
-DCMAKE_EXE_LINKER_FLAGS="--coverage"
165165
@echo "==> Building..."
166166
cmake --build $(BUILD_DIR)/$(CMAKE_PRESET_DEBUG) --parallel $(PARALLEL_JOBS)

cmake/CUDAConfig.cmake

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ add_compile_options(
55
$<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CONFIG:RelWithDebInfo>>:-lineinfo>
66
)
77

8-
# Fast math for Release builds only (slightly reduces precision)
9-
add_compile_options(
10-
$<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CONFIG:Release>>:--use_fast_math>
11-
)
8+
# Fast math for Release builds only (opt-in: flushes denormals, reduces exp/log precision)
9+
option(AC_CUDA_FAST_MATH "Enable --use_fast_math for CUDA Release builds" OFF)
10+
if(AC_CUDA_FAST_MATH)
11+
add_compile_options(
12+
$<$<AND:$<COMPILE_LANGUAGE:CUDA>,$<CONFIG:Release>>:--use_fast_math>
13+
)
14+
endif()
1215

1316
# Extended lambda support (required for modern CUDA C++ patterns)
1417
add_compile_options(

scripts/visualize_dendrite.py

Lines changed: 99 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
# make cuda-visualize VIZ_LAYOUT=single
4343
# make cuda-visualize VIZ_EXTRA_ARGS=--skip-saturated
4444
#
45+
# I/O note: frames are read TWICE — once in the prescan pass (colour ranges,
46+
# saturation flags, time-series data) and once during rendering. Use
47+
# --prescan-limit N to cap the first pass if you have thousands of frames.
48+
#
4549
# Self-test
4650
# python scripts/visualize_dendrite.py --self-test
4751
# Synthesizes a tiny 24³ grid in /tmp, renders one frame, asserts
@@ -138,8 +142,15 @@ def natural_step(path: str) -> int:
138142

139143
def discover_frames(input_dir: Path, pattern: str) -> List[Path]:
140144
"""Return sorted list of snapshot Paths whose name matches the regex."""
141-
matches = sorted(glob.glob(str(input_dir / pattern)), key=natural_step)
142-
return [Path(p) for p in matches if natural_step(p) >= 0]
145+
raw = glob.glob(str(input_dir / pattern))
146+
filtered = [p for p in raw if natural_step(p) >= 0]
147+
n_dropped = len(raw) - len(filtered)
148+
if n_dropped > 0:
149+
sys.stderr.write(
150+
f"WARN: {n_dropped} file(s) matched glob '{pattern}' but had no "
151+
f"parseable step number — skipped\n"
152+
)
153+
return [Path(p) for p in sorted(filtered, key=natural_step)]
143154

144155

145156
# ── Saturation detection ─────────────────────────────────────────────────────
@@ -188,6 +199,14 @@ class ScanResult:
188199
saturated: List[bool] = field(default_factory=list)
189200
grid_dims: Tuple[int, int, int] = (0, 0, 0)
190201
grid_bounds: Tuple[float, float, float, float, float, float] = (0,) * 6
202+
_step_to_idx: Dict[int, int] = field(default_factory=dict, repr=False)
203+
204+
def build_index(self) -> None:
205+
self._step_to_idx = {s: i for i, s in enumerate(self.steps)}
206+
207+
def is_saturated(self, step: int) -> Optional[bool]:
208+
idx = self._step_to_idx.get(step)
209+
return self.saturated[idx] if idx is not None else None
191210

192211

193212
def compute_global_scan(frames: Sequence[Path],
@@ -275,6 +294,8 @@ def compute_global_scan(frames: Sequence[Path],
275294
if scanned == 0:
276295
raise RuntimeError("No readable frames found during prescan")
277296

297+
result.build_index()
298+
278299
# phi is physically in [-1, 1] — clamp for a stable, symmetric colormap.
279300
phi_lo = max(-1.0, phi_lo if math.isfinite(phi_lo) else -1.0)
280301
phi_hi = min(+1.0, phi_hi if math.isfinite(phi_hi) else +1.0)
@@ -520,7 +541,9 @@ def render_slice_panel(grid: pv.StructuredGrid,
520541
bg: Tuple[float, float, float]) -> np.ndarray:
521542
"""Render a single mid-z slice as an RGBA numpy array."""
522543
if scalar not in grid.point_data:
523-
return np.full((window_size[1], window_size[0], 4), 200, dtype=np.uint8)
544+
fb = np.full((window_size[1], window_size[0], 4), 200, dtype=np.uint8)
545+
fb[:, :, 3] = 255
546+
return fb
524547

525548
cz = 0.5 * (grid.bounds[4] + grid.bounds[5])
526549
eps = 0.005 * (grid.bounds[5] - grid.bounds[4])
@@ -529,9 +552,13 @@ def render_slice_panel(grid: pv.StructuredGrid,
529552
origin=(0.0, 0.0, cz + eps))
530553
except Exception as exc:
531554
sys.stderr.write(f"WARN: slice panel ({scalar}) failed: {exc}\n")
532-
return np.full((window_size[1], window_size[0], 4), 200, dtype=np.uint8)
555+
fb = np.full((window_size[1], window_size[0], 4), 200, dtype=np.uint8)
556+
fb[:, :, 3] = 255
557+
return fb
533558
if s.n_points == 0:
534-
return np.full((window_size[1], window_size[0], 4), 200, dtype=np.uint8)
559+
fb = np.full((window_size[1], window_size[0], 4), 200, dtype=np.uint8)
560+
fb[:, :, 3] = 255
561+
return fb
535562

536563
p = pv.Plotter(off_screen=True, window_size=list(window_size))
537564
p.set_background(color=bg)
@@ -607,7 +634,7 @@ def render_timeseries_sidebar(scan: ScanResult,
607634
sat_mask = np.asarray(scan.saturated, dtype=bool)
608635
if sat_mask.any():
609636
sat_steps = steps[sat_mask]
610-
ax.axvspan(sat_steps.min(), steps.max(), color="#ffd2cc",
637+
ax.axvspan(sat_steps.min(), sat_steps.max(), color="#ffd2cc",
611638
alpha=0.55, zorder=0,
612639
label="saturated (wall reached)")
613640

@@ -633,7 +660,13 @@ def render_timeseries_sidebar(scan: ScanResult,
633660
for spine in ("top",):
634661
ax.spines[spine].set_visible(False)
635662
ax.grid(True, alpha=0.25, linestyle=":")
636-
ax.legend(loc="upper left", fontsize=9, framealpha=0.85)
663+
# Combine handles from both axes for a unified legend
664+
handles, labels = ax.get_legend_handles_labels()
665+
if scan.mean_u:
666+
h2, l2 = ax2.get_legend_handles_labels()
667+
handles += h2
668+
labels += l2
669+
ax.legend(handles, labels, loc="upper left", fontsize=9, framealpha=0.85)
637670
else:
638671
ax.text(0.5, 0.5, "no time-series data",
639672
transform=ax.transAxes, ha="center", va="center")
@@ -713,8 +746,7 @@ def render_3d_view(grid: pv.StructuredGrid,
713746

714747
def overlay_saturation_badge(img: Image.Image) -> Image.Image:
715748
"""Burn a red 'SATURATED — wall reached' banner into the upper-right corner."""
716-
out = img.copy()
717-
draw = ImageDraw.Draw(out, "RGBA")
749+
draw = ImageDraw.Draw(img, "RGBA")
718750
text = "SATURATED — wall reached"
719751
font = _load_font(20)
720752
if font is not None:
@@ -723,12 +755,12 @@ def overlay_saturation_badge(img: Image.Image) -> Image.Image:
723755
else:
724756
tw, th = 8 * len(text), 18
725757
pad = 12
726-
x = out.width - tw - 3 * pad
758+
x = img.width - tw - 3 * pad
727759
y = pad
728760
draw.rounded_rectangle((x, y, x + tw + 2 * pad, y + th + 2 * pad),
729761
radius=8, fill=(204, 0, 0, 220))
730762
draw.text((x + pad, y + pad), text, fill=(255, 255, 255, 255), font=font)
731-
return out
763+
return img
732764

733765

734766
def _load_font(size: int) -> Optional[ImageFont.ImageFont]:
@@ -745,6 +777,8 @@ def _load_font(size: int) -> Optional[ImageFont.ImageFont]:
745777
except Exception:
746778
continue
747779
try:
780+
return ImageFont.load_default(size=size)
781+
except TypeError:
748782
return ImageFont.load_default()
749783
except Exception:
750784
return None
@@ -822,6 +856,7 @@ class RenderConfig:
822856
bg_top: Tuple[float, float, float]
823857
silhouette: bool
824858
skip_saturated: bool
859+
saturation_threshold: float = DEFAULT_SATURATION_THRESHOLD
825860

826861

827862
def render_frame(frame_path: Path,
@@ -848,7 +883,9 @@ def render_frame(frame_path: Path,
848883
grid.point_data[field_name] = np.where(np.isfinite(arr), arr, 0.0)
849884

850885
step = natural_step(str(frame_path))
851-
saturated = detect_saturation(grid)
886+
# Use prescan result (computed with the user's --saturation-threshold)
887+
sat_result = scan.is_saturated(step)
888+
saturated = sat_result if sat_result is not None else detect_saturation(grid, cfg.saturation_threshold)
852889
if saturated and cfg.skip_saturated:
853890
sys.stdout.write(f" [{index + 1:>4}/{total}] step={step:<7} "
854891
f"-> SKIPPED (saturated)\n")
@@ -947,17 +984,26 @@ def stitch_video(png_paths: List[Path], mp4_path: Path, fps: int) -> None:
947984
return
948985
mp4_path.parent.mkdir(parents=True, exist_ok=True)
949986
sys.stdout.write(f"==> Stitching {len(png_paths)} frames -> {mp4_path} @ {fps} fps\n")
950-
writer = imageio.get_writer(
951-
str(mp4_path),
952-
fps=fps,
953-
codec="libx264",
954-
quality=8,
955-
pixelformat="yuv420p",
956-
macro_block_size=1,
957-
)
987+
try:
988+
writer = imageio.get_writer(
989+
str(mp4_path),
990+
fps=fps,
991+
codec="libx264",
992+
quality=8,
993+
pixelformat="yuv420p",
994+
macro_block_size=2,
995+
)
996+
except Exception as exc:
997+
sys.stderr.write(
998+
f"ERROR: failed to initialize video writer ({exc}). "
999+
"Ensure ffmpeg is installed and libx264 codec is available.\n"
1000+
)
1001+
return
9581002
try:
9591003
for p in png_paths:
9601004
writer.append_data(imageio.imread(str(p)))
1005+
except Exception as exc:
1006+
sys.stderr.write(f"ERROR: video encoding failed at {p}: {exc}\n")
9611007
finally:
9621008
writer.close()
9631009

@@ -1045,6 +1091,12 @@ def run_self_test(workdir: Optional[Path] = None) -> int:
10451091
return 2
10461092

10471093
sys.stdout.write(f" render ok: {png} ({png.stat().st_size} bytes)\n")
1094+
1095+
# Clean up temporary directory if we created it
1096+
if workdir is None:
1097+
import shutil
1098+
shutil.rmtree(tmpdir, ignore_errors=True)
1099+
10481100
sys.stdout.write("==> self-test PASSED\n")
10491101
return 0
10501102

@@ -1123,7 +1175,9 @@ def parse_args(argv: Optional[List[str]] = None) -> argparse.Namespace:
11231175
help="Only render the first N frames (0 = all)")
11241176
p.add_argument("--prescan-limit", type=int, default=0,
11251177
help="Max frames to scan for global colour range / time-series "
1126-
"(0 = all frames)")
1178+
"(0 = all frames). NOTE: if set, the time-series sidebar "
1179+
"will only show data for the first N frames, and colour "
1180+
"ranges may not cover later frames.")
11271181

11281182
# Bootstrap
11291183
p.add_argument("--no-xvfb", action="store_true",
@@ -1168,12 +1222,26 @@ def main(argv: Optional[List[str]] = None) -> int:
11681222
# ── Resolve layout ───────────────────────────────────────────────────
11691223
layout = args.layout
11701224
if args.view is not None:
1171-
# Legacy compat
1225+
sys.stderr.write(
1226+
f"WARN: --view is deprecated, use --layout instead. "
1227+
f"Mapping --view={args.view} to equivalent --layout.\n"
1228+
)
11721229
if args.view in ("iso", "combined"):
11731230
layout = "single"
11741231
elif args.view == "slice":
11751232
layout = "panels"
11761233

1234+
# Validate window size
1235+
w_w, w_h = args.window_size
1236+
if w_w < 2 or w_h < 2:
1237+
sys.stderr.write(f"ERROR: --window-size must be >= 2, got {w_w}x{w_h}\n")
1238+
return 2
1239+
if w_w % 2 != 0 or w_h % 2 != 0:
1240+
w_w = w_w + (w_w % 2)
1241+
w_h = w_h + (w_h % 2)
1242+
sys.stderr.write(f"WARN: --window-size rounded to even: {w_w}x{w_h}\n")
1243+
args.window_size = [w_w, w_h]
1244+
11771245
try:
11781246
bg_bottom = _parse_color_triplet(args.bg_bottom)
11791247
bg_top = _parse_color_triplet(args.bg_top)
@@ -1245,14 +1313,21 @@ def main(argv: Optional[List[str]] = None) -> int:
12451313
bg_top=bg_top,
12461314
silhouette=not args.no_silhouette,
12471315
skip_saturated=args.skip_saturated,
1316+
saturation_threshold=args.saturation_threshold,
12481317
)
12491318

12501319
out_dir.mkdir(parents=True, exist_ok=True)
12511320
png_paths: List[Path] = []
12521321
ok = 0
1322+
skipped = 0
12531323
for i, f in enumerate(frames):
12541324
step = natural_step(str(f))
12551325
png = out_dir / f"frame_{step:06d}.png"
1326+
if png.exists() and png.stat().st_size > 0:
1327+
png_paths.append(png)
1328+
ok += 1
1329+
skipped += 1
1330+
continue
12561331
t_frame = time.perf_counter()
12571332
success = render_frame(f, png, scan, cfg, index=i, total=len(frames))
12581333
if success:
@@ -1265,6 +1340,8 @@ def main(argv: Optional[List[str]] = None) -> int:
12651340
f"-> {png.name} ({dt_f:.2f}s)\n"
12661341
)
12671342
sys.stdout.flush()
1343+
if skipped:
1344+
sys.stdout.write(f"==> Skipped {skipped} existing frames (resume)\n")
12681345

12691346
sys.stdout.write(f"==> Rendered {ok}/{len(frames)} frames\n")
12701347

src/core/FieldData.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include <algorithm>
44
#include <cstring>
5+
#include <stdexcept>
56

67
namespace ac {
78

@@ -14,7 +15,10 @@ void FieldData::fill(Real value) {
1415
}
1516

1617
void FieldData::copy_from(const Real* src, std::size_t count) {
17-
assert(count <= data_.size());
18+
if (count > data_.size()) {
19+
throw std::out_of_range("FieldData::copy_from: count (" + std::to_string(count) +
20+
") exceeds field size (" + std::to_string(data_.size()) + ")");
21+
}
1822
std::memcpy(data_.data(), src, count * sizeof(Real));
1923
}
2024

src/core/SimulationConfig.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,13 +223,16 @@ SimulationConfig SimulationConfig::from_json(const std::filesystem::path& path)
223223
}
224224
json j = json::parse(file);
225225
auto cfg = parse_config(j);
226+
cfg.validate();
226227
spdlog::info("Configuration loaded from {}", path.string());
227228
return cfg;
228229
}
229230

230231
SimulationConfig SimulationConfig::from_json_string(const std::string& json_str) {
231232
json j = json::parse(json_str);
232-
return parse_config(j);
233+
auto cfg = parse_config(j);
234+
cfg.validate();
235+
return cfg;
233236
}
234237

235238
void SimulationConfig::validate() const {
@@ -291,6 +294,19 @@ void SimulationConfig::validate() const {
291294
// Output
292295
if (output.frequency < 1)
293296
throw std::invalid_argument("output frequency must be >= 1");
297+
if (output.format != "vts" && output.format != "raw") {
298+
throw std::invalid_argument("output format must be 'vts' or 'raw', got: " + output.format);
299+
}
300+
301+
// Checkpoint
302+
if (checkpoint.frequency < 1)
303+
throw std::invalid_argument("checkpoint frequency must be >= 1");
304+
if (checkpoint.keep_last < 1)
305+
throw std::invalid_argument("checkpoint keep_last must be >= 1");
306+
307+
// Initial condition
308+
if (initial.seed_radius <= 0.0)
309+
throw std::invalid_argument("seed_radius must be positive");
294310

295311
// GPU
296312
if (gpu.device_ids.empty())

0 commit comments

Comments
 (0)