Support HDF5 2.0's new H5T_COMPLEX datatype class#72
Closed
brtnfld wants to merge 9 commits into
Closed
Conversation
… test suite) Add a daos-test job to CI that installs DAOS 2.6.5, builds HDF5 from source (matrixed over hdf5_1.14.6 and develop, the latter marked continue-on-error since it tracks a moving upstream target), builds the connector against it, and runs the full ctest suite (h5vl_test plus HDF5's own native/parallel test binaries ported to run against a VOL connector, plus the connector's own h5daos_test_* targets) against a live, ephemeral, single-node DAOS instance. Key pieces: - Rocky Linux 8 container running --privileged (required to disable transparent hugepages, which DAOS's SPDK housekeeping needs even with a pure-RAM storage tier). - test/scripts/daos_server.yml.in: RAM-emulated SCM storage tier sized and configured for small/CI hosts (system_ram_reserved lowered, scm_hugepages_disabled, nr_hugepages fixed). - test/scripts/daos_pool.sh.in: rewritten for the modern dmg CLI syntax, polling for engine join and pool queryability before signaling readiness to the test driver, to fix races where the harness would start the client before the pool was actually usable. - Excludes 3 known, tracked failures (h5_test_testhdf5, h5_partest_t_bigio, h5vl_ext_h5daos_test_metadata_parallel) so CI stays green on everything that's actually been verified so far - see the PR description for root-cause details on each. - .gitmodules: test/vol temporarily points at the brtnfld/vol-tests fork branch pending HDFGroup/vol-tests#67 merging; revert to HDFGroup/vol-tests once that lands.
…ia CI testing Real, confirmed bugs found while exercising the connector against a live DAOS instance for the first time: - daos_vol_file.c: H5Fget_obj_count/H5Fget_obj_ids with H5F_OBJ_ALL compared open objects' container label string instead of the H5_daos_file_t pointer, over-counting when the same file is opened more than once (two independent opens share the same underlying DAOS container). - daos_vol_type.c / daos_vol_private.h / daos_vol_obj.c: H5_daos_dtype_t cached committed datatypes as a persistently-open hid_t, which looks identical to a user-visible open object to HDF5 core's native H5Fget_obj_count(H5F_OBJ_ALL, ...) accounting. Cache the datatype in its serialized (H5Tencode'd) form instead. - daos_vol.c: stop falsely advertising H5VL_CAP_FLAG_EXTERNAL_LINKS and H5VL_CAP_FLAG_UD_LINKS - H5_daos_link_create() hard-errors on H5VL_LINK_CREATE_UD (how H5Lcreate_external() is implemented internally by HDF5), so external/UD links were never actually supported despite the advertised flags. - daos_vol_link.c: reject H5Lcreate_hard() across two different files, matching the existing check already present in the H5Lmove/H5Lcopy path; broaden the creation-order-untracked fallback in H5_daos_link_ibco_task() so H5Ovisit's internal per-group link enumeration (non-recursive, but still needing "best effort" name-order fallback like H5Lvisit) doesn't hard-error when visiting a group without creation-order tracking (e.g. a file's root group). - daos_vol_obj.c: handle H5I_ATTR in the H5VL_OBJECT_BY_SELF target-object resolution used by H5Oget_info and H5Ovisit, instead of type-punning the attribute's H5_daos_item_t header as a full H5_daos_obj_t. HDF5 2.0 (tracked by the develop branch) API compatibility: - daos_vol_private.h: add H5_VERSION_GE-gated macros for H5Tdecode (H5Tdecode1(buf) vs H5Tdecode2(buf, buf_size)) and for looking up this connector's own registered ID by class value (H5VLpeek_connector_id_by_value(), removed in 2.0, vs H5VLget_connector_id_by_value()). Update all 5 H5Tdecode call sites (daos_vol_attr.c, daos_vol_dset.c, daos_vol_map.c, daos_vol_obj.c) to use the new macro.
Points at the merged HDFGroup/vol-tests#67, which fixes test-driver robustness against benign DAOS log noise, gates known DAOS-specific test gaps on connector name rather than skipping unconditionally, and fixes HDF5 2.0 compatibility issues in the vendored test suite; and HDFGroup#68, which fixes an unrelated pre-existing CI workflow typo (ctest --build isn't a valid flag) discovered while investigating that suite's own CI status. .gitmodules is reverted to the upstream vol-tests URL now that both fixes have landed there.
… dispatch HDF5 2.0 (tracked by the develop branch) adds a new datatype class, H5T_COMPLEX (native complex-number support). Two switch statements in daos_vol_type.c enumerate all known H5T_class_t values and error out via "invalid type class" on anything unrecognized: - H5_daos_detect_vl_vlstr_ref(): decides whether a type needs H5Tconvert() (always true for VLEN/REFERENCE, never for plain numeric leaf types like INTEGER/FLOAT). - H5_daos_need_bkg(): decides whether a background buffer is needed for conversion, same shape. Add H5T_COMPLEX to the "no conversion / no background buffer needed" group alongside INTEGER/FLOAT, since it's a fixed-size numeric leaf type from the connector's perspective - HDF5 core does all class-specific interpretation (encoding, conversion) transparently via H5Tencode/ H5Tdecode, which this connector already uses everywhere it stores a datatype, so no other connector code needed to change. Version-gated since H5T_COMPLEX doesn't exist as a symbol pre-2.0.
brtnfld
requested review from
fortnern,
jhendersonHDF and
soumagne
as code owners
H5_daos_dtype_t holds a live, open hid_t for its committed datatype, matching how H5_daos_attr_t, H5_daos_dset_t, and H5_daos_map_t already cache their stored datatypes. These IDs are legitimate app-level HDF5 IDs regardless of whether a VOL connector considers them internal bookkeeping, so H5Fget_obj_count(H5F_OBJ_ALL, ...) counting them is correct behavior, not something to avoid.
Contributor
Author
|
Folded into #73 |
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
1 participant
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.
Summary
HDF5 2.0 (tracked by the
developbranch) adds a new datatype class,H5T_COMPLEX(native complex-number support). Two switch statements indaos_vol_type.cenumerate all knownH5T_class_tvalues and error out via "invalid type class" on anything unrecognized:H5_daos_detect_vl_vlstr_ref(): decides whether a type needsH5Tconvert().H5_daos_need_bkg(): decides whether a background buffer is needed for conversion.Adds
H5T_COMPLEXto the "no conversion / no background buffer needed" group alongsideINTEGER/FLOAT, since it's a fixed-size numeric leaf type from the connector's perspective - HDF5 core does all class-specific interpretation (encoding, conversion) transparently viaH5Tencode/H5Tdecode, which this connector already uses everywhere it stores a datatype. No other connector code needed to change: dataset/attribute/map storage paths never inspect datatype class at all. Version-gated sinceH5T_COMPLEXdoesn't exist as a symbol pre-2.0.Companion fix in the test suite itself (required to actually exercise this, since the shared random-datatype generator didn't know about the new class either): HDFGroup/vol-tests#69.
Related to #69 (tracking HDF5 2.0/develop compatibility work).
.gitmodulestemporarily pointstest/volat thebrtnfld/vol-testsfork branch pending that PR's review; will revert to upstream once merged (same pattern as #71).The two commits that are actually new here
fix: treat H5T_COMPLEX as a plain numeric leaf type in datatype-class dispatch- the only connector source change.test: point test/vol submodule at vol-tests support-h5t-complex fork branch- temporary, pending the companion vol-tests PR.