1.12 New References for Dimension Scales (#1139) #1574
Merged
Conversation
This file contains 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
* Enable usage of new-style references with dimension scale APIs * Add API to check if an object ID represents a native connector object * Modified code to use new function H5DSwith_new_ref to determine if new references should be used with Dimension Scales. The new function return TRUE if non-native connector is used or if H5_DIMENSION_SCALES_WITH_NEW_REF varible is define at configure time (--enable-dimension-scales-with-new-ref). Tested on jelly. ToDo: generate testing file on BE system and enable the test; add flag to CMake; test netCDF-4 with the new references. * Adding new test files generated on BE system (hedgehog) created by 32 and 64-bit library. test_ds chokes on test_ds_le_new_ref.h5 on BE system; test passes for test_ds_be_new_ref-32bit.h5 for the 32-bit library and fails for the 64-bit library, and vice versa. I am checking the files for further investigation; but current implementation of the new references is not portable between LE and BE systems, and 32 and 64-bit systems. * Minor fixes for testing issues * Update test_ds.c Enabled broken test; tests pass now. * Update RELEASE.txt Documented new option to use new references with the HDF5 dimension scales APIs (H5DS*). * Update MANIFEST for new 32-bit new-style references test file for H5DS APIs * Update 'dimension scales w/ new-style refs' feature based on review Co-authored-by: Elena <epourmal@hdfgroup.org>
lrknox
requested review from
jhendersonHDF,
fortnern,
soumagne,
vchoi-hdfgroup,
ChristopherHogan,
gnuoyd,
derobins,
jrmainzer,
bmribler and
byrnHDF
as code owners
derobins
reviewed
Apr 2, 2022
src/H5VL.c
Outdated
| * connector object. | ||
| * | ||
| * Return: Success: TRUE/FALSE | ||
| * Failure: FAIL |
There was a problem hiding this comment.
We need to fix this! This API call needs to be reworked so it returns herr_t and hbool_t is_native is an out parameter (htri_t is trash and should not be used in new code).
There was a problem hiding this comment.
It was changed 5 days later. I've pulled that change in, please review again.
derobins
requested changes
Apr 2, 2022
derobins
approved these changes
Apr 2, 2022
|
I was under the impression that we weren't planning on bringing these changes back to the support branches. Do we have a reason to support it for 1.12? |
|
Is it needed for netCDF-4?
…________________________________
From: jhendersonHDF ***@***.***>
Sent: Saturday, April 2, 2022 12:22 PM
To: HDFGroup/hdf5 ***@***.***>
Cc: Larry Knox ***@***.***>; Author ***@***.***>
Subject: Re: [HDFGroup/hdf5] 1.12 New References for Dimension Scales (#1139) (PR #1574)
I was under the impression that we weren't planning on bringing these changes back to the support branches. Do we have a reason to support it for 1.12?
—
Reply to this email directly, view it on GitHub<#1574 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADGMWCWQGZUCD2IKPYA4KC3VDB65HANCNFSM5SKBWXNQ>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Enable usage of new-style references with dimension scale APIs
Add API to check if an object ID represents a native connector object
Modified code to use new function H5DSwith_new_ref to determine if new references should be used with
Dimension Scales. The new function return TRUE if non-native connector is used or if H5_DIMENSION_SCALES_WITH_NEW_REF
varible is define at configure time (--enable-dimension-scales-with-new-ref).
Tested on jelly.
ToDo: generate testing file on BE system and enable the test; add flag to CMake; test netCDF-4 with the new references.
test_ds chokes on test_ds_le_new_ref.h5 on BE system; test passes for test_ds_be_new_ref-32bit.h5
for the 32-bit library and fails for the 64-bit library, and vice versa. I am checking the files for further
investigation; but current implementation of the new references is not portable between LE and BE systems,
and 32 and 64-bit systems.
Minor fixes for testing issues
Update test_ds.c
Enabled broken test; tests pass now.
Documented new option to use new references with the HDF5 dimension scales APIs (H5DS*).
Update MANIFEST for new 32-bit new-style references test file for H5DS APIs
Update 'dimension scales w/ new-style refs' feature based on review
Co-authored-by: Elena epourmal@hdfgroup.org