Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info #2074

Merged
merged 3 commits into from
Dec 19, 2022

Conversation

mkitti
Copy link
Contributor

@mkitti mkitti commented Aug 27, 2022

Fix #2056

For H5D_chunk_iter_op_t

  • filter_mask changes from type uint32_t to unsigned
  • size changes from uint32_t to hsize_t

The practical change is that size argument is now a 64-bit unsigned integer.

These changes make H5D_chunk_iter_op_t similar the argument similar to the size argument in H5Dget_chunk_info.

H5_DLL herr_t H5Dget_chunk_info(hid_t dset_id, hid_t fspace_id, hsize_t chk_idx, hsize_t *offset,
                                unsigned *filter_mask, haddr_t *addr, hsize_t *size);

The change is important because it allows the internal chunk size to change to be a 64-bit value in the future without modifying the public API.

@derobins
Copy link
Member

The change is important because it allows the internal chunk size to change to be a 64-bit value in the future without modifying the public API.

The problem with this, though, is that using a 64-bit value now makes the function signature misleading until we update the file format. That will definitely not happen in 1.14.0, so we should save this change until HDF5 2.0 (or whenever we get around to updating the file format).

@mkitti
Copy link
Contributor Author

mkitti commented Sep 17, 2022

I find the inconsistency with the rest of the public chunk API more problematic at the present.

The mismatch between hsize_t and the current maximum chunk size has existed for as long as hsize_t became a 64-bit integer.

@derobins derobins changed the title Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info WIP: Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info Dec 17, 2022
@derobins derobins marked this pull request as draft December 17, 2022 04:57
@derobins derobins changed the title WIP: Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info Dec 17, 2022
@derobins
Copy link
Member

I'll approve this and we can merge this in, but:

(A) This HAS to go to 1.12 ASAP since the chunk iteration stuff was merged there (but not released in 1.12.2).
(B) I'm not excited by the types in these functions and we should update them in 2.0 with a chunk dimension type and a fixed-size mask type.

@derobins derobins marked this pull request as ready for review December 17, 2022 05:03
@derobins derobins added merge to 1.12 Merge - To 1.14 This needs to be merged to HDF5 1.14 labels Dec 17, 2022
@mkitti mkitti force-pushed the mkitti/h5dchunk_iter_hsizet_size branch from 1da8185 to 49a75a8 Compare December 19, 2022 04:50
@mkitti
Copy link
Contributor Author

mkitti commented Dec 19, 2022

I rebased on develop since it's been a while.

@derobins derobins merged commit b9244a8 into HDFGroup:develop Dec 19, 2022
derobins added a commit to derobins/hdf5 that referenced this pull request Dec 20, 2022
b9244a8 Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (HDFGroup#2074)
70cf2c3 Removed idioms and misc. text clean-up (HDFGroup#2320)
8102fa8 Only document Fortran functions (HDFGroup#2319)
784061b moved onion VFD to FAPL group (HDFGroup#2321)
6b6bcde Hdffv 11052 (HDFGroup#2315)
10c693a Update hdf5_header.html
0cb5808 Hdffv 11052 (HDFGroup#2303)
a1c81ed added doc. warning for H5Literate_async return value (HDFGroup#2295)
502b32b Updated H5ES documenation (HDFGroup#2293)
a903600 Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (HDFGroup#2291)
mkitti added a commit to mkitti/hdf5 that referenced this pull request Dec 20, 2022
…up#2074)

* Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info

* Modify chunk_info test to for unsigned / hsize_t types

* Fix types in test

* Port of HDFGroup#2074 to hdf5_1_12 branch
@mkitti
Copy link
Contributor Author

mkitti commented Dec 20, 2022

I cherry picked this on the hdf5_1_12 branch: #2329

derobins added a commit to derobins/hdf5 that referenced this pull request Dec 20, 2022
b9244a8 Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (HDFGroup#2074)
70cf2c3 Removed idioms and misc. text clean-up (HDFGroup#2320)
8102fa8 Only document Fortran functions (HDFGroup#2319)
784061b moved onion VFD to FAPL group (HDFGroup#2321)
6b6bcde Hdffv 11052 (HDFGroup#2315)
10c693a Update hdf5_header.html
0cb5808 Hdffv 11052 (HDFGroup#2303)
a1c81ed added doc. warning for H5Literate_async return value (HDFGroup#2295)
502b32b Updated H5ES documenation (HDFGroup#2293)
a903600 Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (HDFGroup#2291)
mkitti added a commit to mkitti/hdf5 that referenced this pull request Dec 20, 2022
…up#2074)

* Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info

* Modify chunk_info test to for unsigned / hsize_t types

* Fix types in test
derobins added a commit that referenced this pull request Dec 21, 2022
b9244a8 Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (#2074)
70cf2c3 Removed idioms and misc. text clean-up (#2320)
8102fa8 Only document Fortran functions (#2319)
784061b moved onion VFD to FAPL group (#2321)
6b6bcde Hdffv 11052 (#2315)
10c693a Update hdf5_header.html
0cb5808 Hdffv 11052 (#2303)
a1c81ed added doc. warning for H5Literate_async return value (#2295)
502b32b Updated H5ES documenation (#2293)
a903600 Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (#2291)
derobins added a commit that referenced this pull request Dec 21, 2022
* Brings the following changesets over from develop:

b9244a8 Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (#2074)
70cf2c3 Removed idioms and misc. text clean-up (#2320)
8102fa8 Only document Fortran functions (#2319)
784061b moved onion VFD to FAPL group (#2321)
6b6bcde Hdffv 11052 (#2315)
10c693a Update hdf5_header.html
0cb5808 Hdffv 11052 (#2303)
a1c81ed added doc. warning for H5Literate_async return value (#2295)
502b32b Updated H5ES documenation (#2293)
a903600 Fix for HDFFV-11052: h5debug fails on a corrupted file (h5_nrefs_POC)… (#2291)

* Brings the following changes over from develop:

c1e44d3 Fix doxygen warnings and remove javadocs (#2324)
149b8e9 Disable hl tools by default (#2313)
derobins pushed a commit that referenced this pull request Feb 21, 2023
* Backport H5Dchunk_iter to 1.10 branch

* Add some accessory files, test still needs work

* Apply proper formatting, and fix compile errors

* Remove const from H5D__chunk_iter as per #1700

* Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (#2074)

* Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info

* Modify chunk_info test to for unsigned / hsize_t types

* Fix types in test

* Add test_basic_query, helper functions to test/chunk_info.c 1_10

* H5Dchunk_iter now passes offsets in units of dataset elements, fix #1419 (#1969)

* H5Dchunk_iter now passes chunk dimension scaled offsets, fix #1419

* Update docs for H5Dchunk_iter, H5Dget_chunk_info*

Modified description for `H5Dchunk_iter`, `H5Dget_chunk_info`, and `H5Dget_chunk_info_by_coord` to the following

 * offset          Logical position of the chunk’s first element in units of dataset elements
 * filter_mask  Bitmask indicating the filters used when the chunk was written
 * size             Chunk size in bytes, 0 if the chunk does not exist

* Fix regression of #1419

* Add a note about return fail in 1.12 and older for invalid chunk index

* Committing clang-format changes

* Run clang-format on test/chunk_info.c

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
brtnfld pushed a commit to brtnfld/hdf5 that referenced this pull request May 17, 2023
…up#2074)

* Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info

* Modify chunk_info test to for unsigned / hsize_t types

* Fix types in test
brtnfld pushed a commit to brtnfld/hdf5 that referenced this pull request Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge - To 1.14 This needs to be merged to HDF5 1.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] H5D_chunk_iter_op_t exposes size (nbytes) as uint32_t rather than hsize_t
3 participants