Conversation
Co-authored-by: Noah Lane Lewis <nlewi26@login35.chn.perlmutter.nersc.gov>
* temporarily allow develop test to fail * Committing clang-format changes * fix runners * dump CI versions --------- Co-authored-by: github-actions <github-actions[bot]@users.noreply.github.com>
* include HDF5 2.0.0 in CI * use develop branch for VOL ASYNC * fix cmake version * update OpenPMD to support HDF5 2.0.0 * set policy * try to fix OpenPMD build * fixes * update OpenPMD * update OpenPMD * store the log * debug CI * update E3SM version * change in E3SM build * change in E3SM build * change in E3SM build * change in E3SM build * change in MACSIO build * fix typo * re-enable other tests and fix develop testing
jeanbez
added
bug
All 19 sprintf sites in h5bench source files now pass an explicit sizeof(buffer) limit. Inputs are currently bounded
integers or already-bounded path components, but the untruncated sprintf form left no margin if upstream buffers ever get reused with larger formats.
Sites (8 files, 19 calls):
- h5bench_patterns/{write,read,overwrite,append,write_unlimited, write_normal_dist}.c: 10 calls. Six grp_name ("Timestep_%d") and three mpi_rank_output_file_path ("%s/rank_%d_%s") formats, plus one read-path grp_name.
- exerciser/h5bench_exerciser.c: 4 calls (testFileName, attr1Buf, attributeName, dataSetName1).
- metadata_stress/hdf5_iotest.c: 6 calls into the 255-byte path buffer (step=%d, array=%d, and the step/array compound format, each appearing twice in write/read phases - collapsed via replace_all).
Vendored submodules (amrex, e3sm, macsio) intentionally not touched.
) * Clean up build-type handling, bump CMake min, sync versions to 1.6 * CMakeLists.txt: raise cmake_minimum_required 3.10 -> 3.13 (CMP0077 floor; AMReX/OpenPMD self-enforce higher when enabled); project VERSION 1.3.0 -> 1.6.0; replace unconditional set(CMAKE_BUILD_TYPE Debug) with a default-Release gate that honours user-supplied -DCMAKE_BUILD_TYPE and multi-config generators. * docker/ubuntu-24.04-hdf5-2.0.0/Dockerfile: fix mislabelled base image (ubuntu:focal -> ubuntu:24.04) and kitware apt codename (focal -> noble). * CITATION.cff / RELEASE.txt: sync release metadata to v.1.6 (commit + date). * spack/packages/h5bench/package.py: add versions 1.3-1.6; fix "depecrated" typo (now deprecates 1.0-1.2). * Added --break-system-packages to pip command in ubuntu-24 Dockerfile. The new version of pip on ubuntu 24 throws an error when you try to install with pip outside a venv. Add the the flag forces global install. --------- Co-authored-by: Isaiah Honor <bonkdroid@gmail.com>
…) (#160) * Extend fail-fast malloc/HDF5 checks to remaining pattern binaries * Committing clang-format changes * Re-introduced assert(ts) --------- Co-authored-by: github-actions <github-actions[bot]@users.noreply.github.com> Co-authored-by: Isaiah Honor <bonkdroid@gmail.com>
…#156) (#162) Wave B item 10. E3SM-IO upstream is autotools-only, so we keep the ExternalProject_Add wrapper, but clean up four long-standing issues: 1. Drop BUILD_COMMAND "make -j 1". Replace with BUILD_COMMAND $(MAKE) so the outer make's jobserver forwards -jN into the E3SM build. CI invokes the outer build with `make -j 2`; until now every job on the matrix was serialising the ~30+ TU E3SM tree. 2. BUILD_IN_SOURCE 0 + explicit BINARY_DIR. Keeps object files, Makefiles, config.log, etc. out of the submodule worktree. autoreconf still writes configure / Makefile.in into the source tree - that's an autotools limitation, not something we can fix in CMake. 3. Only pass CFLAGS=-fno-var-tracking-assignments (and the matching CXXFLAGS) when CMAKE_C_COMPILER_ID is GNU. That flag is a gcc-only debug-info-slowdown workaround; Clang warns or ignores it. 4. Add install(PROGRAMS ${CMAKE_BINARY_DIR}/h5bench_e3sm DESTINATION bin). Previously the INSTALL_COMMAND only copied the binary into the CMake build dir; `cmake --install` (or `make install`) left the E3SM binary behind. CI tests happen to run from the build dir so they masked this gap. Pinning the e3sm submodule to a specific upstream tag is the separate item 12 and covers all four submodules together. Verified locally (CMake 4.3.1, clang) that configure still succeeds and the baseline + exerciser + metadata targets build unchanged; the generated h5bench_e3sm build step now uses cmake -Dmake=$(MAKE) -P h5bench_e3sm-build-Release.cmake so the jobserver handoff is literal-$(MAKE)-as-expected.
Mirrors the HDF5 2.0.0 setup. New ubuntu-24.04-hdf5-2.1.1 Dockerfile based on Ubuntu 24.04, building HDF5 2.1.1 from the GitHub release tarball with the same parallel + threadsafe + allow-unsupported flags used for 2.0.0. New h5bench-hdf5-2.1.1.yml workflow consumes hpcio/hdf5-2.1.1 and runs the same SYNC + ASYNC suite as the 2.0.0 job.
…t scope (#164) * Fix invalid allow_failure key that disabled the develop CI workflows h5bench-hdf5-develop-test.yml and h5bench-hdf5-develop.yml both carried a job-level `allow_failure: true`. That is a GitLab CI keyword — GitHub Actions does not recognize it, and an unknown job key makes GitHub reject the entire workflow file as invalid ("This run likely failed because of a workflow file issue", zero jobs, conclusion failure). The result: neither develop workflow has actually run since the key was introduced in 5ff4c5b ("Temporarily allow tests with HDF5 develop branch to fail (#144)"). h5bench-hdf5-develop-test.yml is the *only* workflow that runs h5bench's own ctest suite (cd build-sync && ctest; cd build-async && ctest), so the entire pytest/ctest test surface — including the new test_schema.py and every test_*.py — has had no CI coverage on pull requests this whole time. GitHub's equivalent of GitLab's allow_failure is continue-on-error. At job scope `continue-on-error: true` lets the job fail without failing the overall run, which is exactly what #144 intended for the still-unstable HDF5 develop branch. After this fix the develop-test workflow parses, triggers on pull_request as designed, and runs the ctest suite again. * Fix HDF5-from-source build in the develop CI workflows Both develop workflows build HDF5 from the HDFGroup/hdf5 develop branch (there is no prebuilt container for it, unlike the versioned workflows which use hpcio/hdf5-<ver> images). The build step did: mkdir hdf5/build cd hdf5/build mkdir build cd build cmake ... .. The doubled `mkdir build && cd build` lands cmake in hdf5/build/build/, so `..` resolves to hdf5/build — which has no CMakeLists.txt: CMake Error: The source directory ".../hdf5/build" does not appear to contain CMakeLists.txt. The hpcio/hdf5-2.0.0 image's Dockerfile builds HDF5 the correct way (cd hdf5; mkdir build; cd build; cmake .. ..), i.e. one build directory with the source root one level up. Removing the spurious inner mkdir/cd makes `cd hdf5/build` the build dir and `..` the hdf5/ source root, matching that recipe. This was masked until now: the develop workflows carried an invalid `allow_failure` key (fixed in the previous commit) that made GitHub reject the whole file, so the HDF5 build never actually executed and nobody saw it was broken. Also adds `-DCMAKE_POLICY_VERSION_MINIMUM=3.5` to the VOL-ASYNC cmake invocation in both develop workflows, matching the change already made to h5bench-hdf5-2.0.0.yml — newer CMake hard-errors on the sub-3.5 cmake_minimum_required some of the VOL-ASYNC build declares, and the flag is a harmless no-op on older CMake. * Set openPMD_* options before add_subdirectory so they take effect The H5BENCH_OPENPMD block set openPMD_BUILD_TESTING, openPMD_BUILD_CLI_TOOLS, openPMD_USE_PYTHON, openPMD_USE_ADIOS*, etc. *after* add_subdirectory(openpmd). CMake evaluates a subproject's CMakeLists.txt at the add_subdirectory call, so every one of those `set(... CACHE ... FORCE)` lines landed too late and openPMD configured itself with its own defaults. Visible symptom: openPMD built its CLI tools and registered their ctest tests (CLI.help.pipe.py, CLI.py.help.ls). Those tests then ran under h5bench's own `ctest` invocation in the develop-test workflow and failed (2 of 15 ctest entries), because they exercise the openpmd-pipe / openpmd-ls Python CLI which h5bench neither wants nor sets up. This was invisible until now: the develop-test workflow — the only CI job that runs h5bench's ctest — had been disabled by an invalid `allow_failure` key, so h5bench's ctest with H5BENCH_OPENPMD on had not actually executed in CI. Reviving the workflow surfaced the latent ordering bug. Moving add_subdirectory(openpmd) below the set() block makes all the openPMD_* directives effective: no CLI tools, no openPMD test registration, and ADIOS/JSON/Python correctly disabled (previously openPMD was also building those subsystems against h5bench's intent). * Scope develop-test ctest to h5bench tests; revert openPMD reorder Two related changes to get the develop-test workflow green. 1. Revert the openPMD set()-before-add_subdirectory reorder. Moving the set(openPMD_* CACHE FORCE) block ahead of add_subdirectory(openpmd) made those directives effective for the first time — and that broke the build. h5bench sets phantom option names (openPMD_USE_JSON, openPMD_USE_ADIOS1) that openPMD doesn't define, and openPMD's PYTHON/MPI/HDF5/ADIOS2 options are tri-state AUTO values created by its openpmd_option() macro, not plain BOOLs. With the block effective, openPMD entered superbuild mode, fetched pybind11, and tried to build its Python bindings — which fails on the runner with "fatal error: Python.h: No such file or directory" (no python3-dev installed). The block has been dead code (positioned after add_subdirectory) for a long time and the build worked fine that way, because openPMD's AUTO defaults resolve sensibly. Properly rewiring the h5bench<->openPMD option passing is its own task; for now restore the known-good ordering. 2. Scope the develop-test ctest run to h5bench-* tests. add_subdirectory(openpmd) (and amrex) register those projects' own ctest suites into the h5bench build tree. `ctest --verbose .` ran them too — openPMD's CLI tests (CLI.help.pipe.py, CLI.py.help.ls) then failed under h5bench's test step. Every test h5bench itself registers in tests/CMakeLists.txt is named h5bench-*, so `ctest -R '^h5bench-'` runs exactly this repository's suite and ignores vendored-subproject tests. Applied to both the SYNC and ASYNC test steps. This keeps the openPMD test-leak fixed without depending on the fragile option-passing, and unbreaks the build.
…ion, Codecov workflow (#157) * Wave C-prep: HDF5 output-correctness test scaffolding Before refactoring commons/ and h5bench_patterns/ in Wave C we need tests that catch silent data corruption, not just silent-exit-zero runs. The existing pytest suite only asserts that a sample config file exists on disk; the VALIDATE flag only greps stdout for a mode string. This patch adds a real content-validation layer. New structure tests/helpers/h5_assert.py — h5py-based assertion library: list_timestep_groups, dataset_sha256, assert_timestep_count, assert_contig_datasets_present, assert_dataset_shape, assert_dataset_dtype, assert_value_in_range, assert_unlimited_axis, assert_id_1_1d_pattern, assert_id_2_1d_write_pattern, assert_datasets_equal. tests/fixtures/tiny-*.json — 1024-particle, 2-rank, 1-2-timestep configs. These are test-owned (do not touch the production samples/) and run in a few seconds each. tests/test_output_validation.py — exercises write / append / overwrite / write-unlimited patterns; checks group + dataset presence, shape, dtype, value ranges, the deterministic id_1 / id_2 patterns on the 1D contig write, H5S_UNLIMITED axis survival, and the "overwrite actually overwrites id_2" invariant. tests/test_read_roundtrip.py — closes the h5bench_read gap: runs write-then-read through the driver and verifies the file survives the read step with the expected structure and id_1 pattern. tests/requirements.txt — h5py, numpy. Infrastructure changes tests/conftest.py — extracted the VOL-path injection into a helper and glob the new tests/fixtures/ alongside the legacy samples/ so both feed through identical pre-processing. tests/CMakeLists.txt — register h5bench-output-validation and h5bench-read-roundtrip with ctest, mirroring the existing h5bench-sync convention. A few subtleties worth noting: * mpi.configuration is respected verbatim by the driver, so the fixtures include "-np 2" explicitly rather than relying on mpi.ranks (which is only consulted when configuration is empty). * h5bench_write overwrites X_DIM/Y_DIM/Z_DIM at runtime with X_RAND/Y_RAND/Z_RAND (191/1009/3701), so the value-range assertion uses those; this is an odd but real part of the current contract Wave C must preserve. * The append step does not grow the file beyond 2 * dim_1: with 2 ranks writing dim_1 particles via write-unlimited first, the file already has extent 2 * dim_1, so append semantically overwrites the second half. The test asserts exactly that invariant. Verified locally (HDF5 2.1.1, macOS / AppleClang): 5 new tests pass; 24 existing sync tests still pass. No behaviour change for CI until this lands as its own PR and tests/requirements.txt gets installed in the workflows - intentionally deferred. * Wave C-prep follow-up: coverage-boosting fixtures and tests Extends the Tier 1 scaffolding with fixtures that drive the three remaining MEM_PATTERN x FILE_PATTERN combinations of h5bench_write, 2D + 3D contig variants, and the previously-untested h5bench_write_var_normal_dist binary. New fixtures (tests/fixtures/) tiny-1d-contig-interleaved.json -> CONTIG mem / INTERLEAVED file tiny-1d-interleaved-contig.json -> INTERLEAVED mem / CONTIG file tiny-1d-interleaved-interleaved.json -> INTERLEAVED mem / INTERLEAVED file tiny-2d-write.json -> NUM_DIMS=2, 32x16 per rank tiny-3d-write.json -> NUM_DIMS=3, 8x8x8 per rank tiny-write-var-normal-dist.json -> write_var_normal_dist, stdev 128 New / extended tests test_output_validation.py -> + 3 compound/interleaved tests, + 2D and 3D structural tests test_write_var_normal_dist.py -> first real pytest for the h5bench_write_var_normal_dist binary (the existing test_sync_h5bench_normal_dist.py has two latent bugs: wrong binary name and non-existent sample filename, so it has never actually run) h5_assert.py gains assert_compound_dataset_present() for the compound-type output variant (single /Timestep_N/particles dataset with an 8-field compound HDF5 type). CMakeLists.txt registers h5bench-write-var-normal-dist with ctest. Coverage (measured with gcovr + coverage.py against an instrumented local build, 35 tests run): C lines: 55.7% -> 64.0% (+8.3) C functions: 59.7% -> 69.5% (+9.8) C branches: 39.2% -> 45.3% (+6.1) write_normal_dist: 0% -> 51% Python driver: 37% -> 37% (flat - expected; these tests all exercise the run_pattern dispatcher) The remaining C gaps live behind USE_COMPRESS, HAVE_SUBFILING, and the ASYNC VOL code paths; Python gains require tests for the non-pattern benchmarks (exerciser / metadata / amrex / openpmd / e3sm / macsio) and negative driver paths (validate_json failures, missing binary). * Wave C-prep: pure-Python driver unit tests Adds tests/test_driver_unit.py with 30 pytest cases for the parts of src/h5bench.py that don't need a benchmark binary: * validate_json: all 5 required keys, each missing-key path exits with EX_DATAERR, extra keys tolerated. * prepare_parallel: mpirun + mpiexec + srun + explicit-configuration + unknown-command branches. * is_available: shutil.which hit in PATH vs. miss. * check_parallel: plain shell vs. $SHELL containing mpirun|mpiexec| srun (which exits with EX_USAGE) vs. $SHELL unset. * prepare_vol / enable_vol / disable_vol / reset_vol: None input, library + preload + path all present, connector key optional, disable when connector unset, reset clears HDF5_PLUGIN_PATH + HDF5_VOL_CONNECTOR + ABT_THREAD_STACKSIZE. * check_for_hdf5_error: clean stderr returns False, HDF5 error banner triggers sys.exit(EX_IOERR). * run() negative paths: missing config file -> EX_NOINPUT, malformed JSON -> EX_NOINPUT. CMakeLists.txt registers h5bench-driver-unit with ctest. Coverage (measured with exerciser + metadata binaries built; 67 tests): Python src/h5bench.py 37% -> 54% (+17) C lines 64.0% -> 64.6% (+0.6) C functions 69.5% -> 72.5% (+3.0) The remaining Python gap (46%) is almost entirely the optional-benchmark dispatchers (run_amrex / run_openpmd / run_e3sm / run_macsio) which need those submodules built, main()/argparse entry point, Lustre detection, and ASYNC-mode sub-branches. 30 new unit tests run in ~0.05s total - no MPI, no HDF5, no subprocess. * Detect CMake options and gate optional-benchmark tests on them Introduces a build-time option introspection mechanism and rewires the optional benchmark tests (AMReX, OpenPMD, E3SM, MACSio, ASYNC VOL, exerciser, metadata) to key off it. Previous behaviour: tests globbed for a binary file and skipped on "not found" - which hid the difference between "option was off" (expected) and "option was on but the binary failed to build" (bug). src/h5bench_configuration.py.in gains an __options__ dict populated by CMake's configure_file substitution at configure time. Every h5bench flag + WITH_ASYNC_VOL is surfaced as a real Python bool: __options__ = { "H5BENCH_METADATA": _flag("@H5BENCH_METADATA@"), "H5BENCH_EXERCISER": _flag("@H5BENCH_EXERCISER@"), "H5BENCH_AMREX": _flag("@H5BENCH_AMREX@"), "H5BENCH_OPENPMD": _flag("@H5BENCH_OPENPMD@"), "H5BENCH_E3SM": _flag("@H5BENCH_E3SM@"), "H5BENCH_MACSIO": _flag("@H5BENCH_MACSIO@"), "WITH_ASYNC_VOL": _flag("@WITH_ASYNC_VOL@"), } tests/helpers/build_config.py exposes a requires_<thing> pytest.mark. skipif for each entry. The reason string always includes the flag name and state, so `pytest -v` output is self-explanatory, e.g.: SKIPPED (H5BENCH_E3SM=OFF at build time (E3SM-IO benchmark)) If h5bench_configuration can't be imported (running pytest out of tree without cmake), the helper falls back to "everything off" so tests skip cleanly instead of erroring. Refactored tests now use the markers and upgrade binary-missing from a silent skip to a hard assert: if H5BENCH_AMREX=ON but h5bench_amrex_sync is missing, that's a build regression and the test fails loudly rather than hiding. test_sync_amrex.py -> @requires_amrex test_async_amrex.py -> @requires_amrex + @requires_async_vol test_sync_openpmd.py -> @requires_openpmd test_async_h5bench.py -> @requires_async_vol test_sync_exerciser.py -> @requires_exerciser test_sync_metadata.py -> @requires_metadata New tests that didn't exist before: test_sync_e3sm.py -> @requires_e3sm test_sync_macsio.py -> @requires_macsio (CMake registers both with ctest, gated on the matching option.) Verified locally against an EXERCISER+METADATA-only build: 66 pattern / unit / metadata tests pass, 23 optional-dep tests skip with explicit per-flag reasons, and one pre-existing macOS-specific h5bench_exerciser SIGTRAP on sync-exerciser.json - unrelated to this change. * Add Codecov coverage workflow for C + Python New .github/workflows/coverage.yml collects line + branch coverage for the h5bench C benchmarks and the Python driver, then uploads both reports to Codecov. Single HDF5 version (1.14.0) with every optional benchmark and VOL-ASYNC enabled, so the one workflow reflects the whole coverage surface rather than duplicating instrumentation across nine per-HDF5-version workflows. Triggers * pull_request to develop or master - PR-level coverage signal * push to master - baseline after merge * workflow_dispatch - manual re-run * schedule, Mondays 06:00 UTC - weekly drift check Build + test flow * Installs coverage tooling (gcovr, coverage.py, h5py, numpy) into an isolated venv so PEP 668 base images (Ubuntu 24.04+) don't block pip. * Clones + builds Argobots and VOL-ASYNC (v1.7) to /opt, same recipe the production workflow uses. * One h5bench CMake configure with -DH5BENCH_ALL=ON + -DWITH_ASYNC_VOL:BOOL=ON + --coverage on CFLAGS/CXXFLAGS and EXE/SHARED linker flags so every compilation unit is instrumented (including AMReX/OpenPMD sub-builds pulled in via add_subdirectory). * Drives the whole tests/ tree via `coverage run ... -m pytest` so the new Wave C-prep tests (output validation, read round-trip, write-var-normal-dist, driver unit, build-config-gated submodule tests) all fire; optional-dep tests that did skip on my laptop will run here because the submodules are built. Coverage collection * gcovr emits coverage-c.xml (Cobertura) + coverage-c.html (detail) filtered to h5bench-owned directories (commons/, h5bench_patterns/, exerciser/, metadata_stress/) so vendored submodule coverage doesn't dominate the numbers. * coverage.py emits coverage-python.xml scoped to src/ (the driver). * codecov/codecov-action@v4 uploads both with -Fc / -Fpython flags so the Codecov UI can present C vs Python breakdowns. * CODECOV_TOKEN is read from secrets but the action tolerates it being empty (tokenless upload still works for public repos). * actions/upload-artifact@v4 archives the HTML + XML reports as a CI artifact (14-day retention) for manual inspection when the Codecov upload itself fails. Local baseline (without submodule builds, recorded on wave-c-prep-tests): C lines 55.7% -> 64.6% C func 59.7% -> 72.5% Python 37% -> 54% With H5BENCH_ALL + WITH_ASYNC_VOL this workflow should push both past 80% because it exercises the run_amrex / run_openpmd / run_e3sm / run_macsio dispatchers and the MODE=ASYNC branches. * coverage CI: drop apt-get, install python tooling via pip --user The hpcio/hdf5-1.14.0 base image (Ubuntu 20.04) carries a Kitware apt source whose GPG key has rotated; running `apt-get update` inside container fails with: W: GPG error: https://apt.kitware.com/ubuntu focal InRelease: NO_PUBKEY E: The repository '...' is not signed. Error: Process completed with exit code 100. We were only running apt to install python3-pip + python3-venv. The image ships python3 + pip already, and Ubuntu 20.04 predates PEP 668's externally-managed-environment marker, so we don't need a venv at all — `pip install --user` works and writes to /root/.local. Putting $HOME/.local/bin on PATH makes the subsequent gcovr/coverage/pytest invocations resolve to the pinned versions. Mirrors the workaround already used in tasks/run_coverage_in_docker.sh when reproducing the workflow locally.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #146 +/- ##
=========================================
Coverage ? 58.61%
=========================================
Files ? 14
Lines ? 6036
Branches ? 1421
=========================================
Hits ? 3538
Misses ? 1923
Partials ? 575
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sync the develop branch to master.