[codex] Wrap libneo hdf5_tools in KAMEL

[krystophny]

Summary

• replace the vendored common/hdf5_tools implementation with a thin compatibility layer over canonical LIBNEO::hdf5_tools
• keep KAMEL_hdf5_tools as the local module surface so KIM and QL-Balance do not need a broad call-site rewrite
• make every KAMEL subproject share one libneo build tree and module include directory

Why

KAMEL was carrying its own divergent hdf5_tools implementation.

This PR keeps the KAMEL-facing module name stable, but moves the real implementation back to canonical libneo code. The compatibility layer only preserves the legacy specific entry points that QL-Balance still links against directly.

Linked work

• libneo issue: Dependency audit: downstream map, duplication report, and split proposals libneo#258
• sparse_draft sibling PR: https://github.com/itpplasma/sparse_draft/pull/15
• benchmark_orbit sibling PR: [codex] Discover current SIMPLE libneo build outputs benchmark_orbit#1

Verification

cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DINSTALL_KIM_SYMLINK=OFF -DPython_EXECUTABLE=/usr/bin/python3
cmake --build build -j$(nproc)
ctest --test-dir build --output-on-failure

Result: KIM.x, ql-balance.x, and fouriermodes.x build successfully, and ctest passes 18/18 after the full build.

[slopqueue]

ORCHESTRATE_REVIEW_STATUS: request_changes

Confirmed: the old h5_append_double_1 had the same SIZE_T/HSIZE_T pattern (line 571: h5sselect_hyperslab_f(..., dims, ...) with dims as SIZE_T). Pre-existing, not introduced here. The new code is actually more correct — explicit int(..., kind=SIZE_T) conversions (lines 38-40) replace the old implicit integer-to-SIZE_T conversions (size = (/shape(value), offset/)).

Here is the final review:

---

{
  "review_status": "approve",
  "confidence": 0.92,
  "summary": "Clean replacement of vendored hdf5_tools with thin compatibility shim over LIBNEO::hdf5_tools. 573→56 lines. All symbols re-exported correctly. Golden + build CI green. One informational finding on float32→float64 on-disk format change in h5_add_float_1, validated by golden test.",
  "requirements": [
    {"id": "REQ-001", "status": "satisfied", "evidence": "CI build-and-test: PASS (5m10s)"},
    {"id": "REQ-002", "status": "satisfied", "evidence": "CI golden-record: PASS (8m32s) — A/B comparison vs golden-baseline tag, rtol 1e-8"},
    {"id": "REQ-003", "status": "satisfied", "evidence": "common/hdf5_tools/hdf5_tools.f90:5 — `use hdf5_tools` + `public` re-exports all libneo symbols"},
    {"id": "REQ-004", "status": "satisfied", "evidence": "All KAMEL callers use `KAMEL_hdf5_tools` module name, unchanged. No caller edits needed."},
    {"id": "REQ-005", "status": "satisfied", "evidence": "CMakeLists.txt: KAMEL_LIBNEO_BINARY_DIR set as CACHE INTERNAL, used consistently in PreProc/fourier, QL-Balance, common/equil"},
    {"id": "REQ-006", "status": "satisfied", "evidence": "No short-circuit .and./.or. in new code. h5_check() calls are sequential, not guarded."},
    {"id": "REQ-007", "status": "satisfied", "evidence": "h5_append_double_1: explicit int(...,kind=SIZE_T/HSIZE_T) conversions (lines 38-40). Old code used implicit conversions. New is more correct."}
  ],
  "findings": [
    {
      "id": "INFO-001",
      "severity": "informational",
      "category": "behavior",
      "file": "common/hdf5_tools/hdf5_tools.f90",
      "line": 20,
      "evidence": "h5_add_float_1 delegates to h5_add_double_1 with dble(). On-disk type changes from H5T_NATIVE_FLOAT (32-bit) to H5T_NATIVE_DOUBLE (64-bit). Old code used h5ltmake_dataset_float_f.",
      "why_blocking": "Not blocking. float32→float64 is lossless. Golden test passes because compare.py:compare_arrays (line 137) uses np.allclose on values, not byte-level format. No downstream consumers check dataset type (verified: Python readers don't read these datasets). File size doubles for these datasets but impact is negligible.",
      "suggested_fix": "Document in PR description that h5_add_float_1 now writes float64 instead of float32. Consider adding a comment in hdf5_tools.f90 noting the precision change."
    },
    {
      "id": "INFO-002",
      "severity": "informational",
      "category": "test",
      "file": "common/hdf5_tools/hdf5_tools.f90",
      "line": 12,
      "evidence": "h5_add_float_1 and h5_append_double_1 have no dedicated unit tests. Only validated through golden integration test (QL-Balance time_evolution.f90 calls h5_add_float_1 8x, toroidal_torque.f90 calls h5_append_double_0).",
      "why_blocking": "Not blocking for a refactoring PR. Golden test covers both functions end-to-end. Isolated unit tests would catch regressions earlier but aren't required for merge.",
      "suggested_fix": "Add test_hdf5_tools.f90 with isolated tests for h5_add_float_1 (verify dataset type, shape, values) and h5_append_double_1 (verify append correctness, offset handling). Register in CMakeLists.txt with add_test."
    },
    {
      "id": "INFO-003",
      "severity": "informational",
      "category": "portability",
      "file": "common/hdf5_tools/hdf5_tools.f90",
      "line": 48,
      "evidence": "h5sselect_hyperslab_f receives dims (integer(SIZE_T)) where API expects integer(HSIZE_T). Pre-existing: old code (golden-baseline:571) has same pattern. Works on all supported platforms (SIZE_T == HSIZE_T == integer(8) on Apple Silicon and Debian GNU).",
      "why_blocking": "Not blocking. Pre-existing issue, not introduced by this PR. New code is actually more correct overall (explicit int conversions) than old code (implicit conversions).",
      "suggested_fix": "None required for this PR. Future cleanup: declare a separate HSIZE_T copy of dims for the h5sselect_hyperslab_f call."
    }
  ],
  "escalation_required": false,
  "escalation_reason": ""
}

---

Verdict: APPROVE.

PR #124 is a clean, well-scoped refactoring. Replaces 573 lines of vendored hdf5_tools with a 56-line compatibility shim over LIBNEO::hdf5_tools. All 6 changed files are focused and correct. CI is green (build-and-test PASS, golden PASS). The one behavioral change (h5_add_float_1 writing float64 instead of float32) is lossless, validated by the golden A/B test, and has no downstream impact. The new h5_append_double_1 is actually more correct than the old version (explicit int(..., kind=) conversions vs implicit). No blockers.

[slopqueue]

Review verdict: Comment

Here's what I can see from the repo state:

Current branch: dep-audit/wave-02-hdf5tools (clean working tree, no PR open)

Work committed on this branch:

1. efc837e8 — Wrap libneo hdf5_tools in KAMEL (replaced ~1700 lines of custom HDF5 code with a thin shim)
2. b05e2f99 — Added unit tests for h5_add_float_1 / h5_append_double_1 and fixed a SIZE_T/HSIZE_T type mismatch on append dims

The diff vs main shows 6 files touched across common/hdf5_tools/, QL-Balance/, common/equil/, PreProc/fourier/, and the top-level CMakeLists.txt.

I don't have prior conversation context, so I can't tell what the next planned step was. Reasonable options:

• Open a PR for this branch (it looks complete: code + tests + type fix)
• Continue the dependency audit to a next wave/target
• Something else you had in mind

What would you like to do?