Skip to content

Fix for issue #6365#6469

Open
vchoi-hdfgroup wants to merge 7 commits into
HDFGroup:developfrom
vchoi-hdfgroup:develop
Open

Fix for issue #6365#6469
vchoi-hdfgroup wants to merge 7 commits into
HDFGroup:developfrom
vchoi-hdfgroup:develop

Conversation

@vchoi-hdfgroup

Copy link
Copy Markdown
Contributor

Clarify the description for the chunked dataset's dimensions.

</tr>
<tr align="center">
<td colspan="4">Dimension 0 Size</td>
<td colspan="4">Dimension 1 Size</td>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these stay 0-based and instead "Dimension #n" below should be changed to n - 1? Otherwise, this seems to conflict with the text below that uses "(n - 1)" when referring to dimensions stored.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at the field "dimensionality" description, "For chunked storage, the value stored is one greater than the dataset’s actual number of dimemsions. For example, a 1-dimensional dataset stores a value of 2. This increment accounts for the dataset element size which occupies the final dimension entry."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But assuming indices are still 0-based here, what's stored is the sizes for dimension 0..(n-1) and the dataset element size just happens to be an extra element in that "array".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you mentioned above conflicting with the text "(n-1)" which line are you referring to?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this diff it's line 7700. That text pretty much states what I'd expect: For an n-dimensional dataset (for n >= 2), dimensions 0..(n-1) are stored and then the dataset element size is stored. The dataset element size is part of the dimensions "array", but it's not really a dimension itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So actually I should have the following:
In the layout table:
"Dimension 0 Size, Dimension 1 size,.... Dimension (n-1) size, Dataset element size"

And in the description table:

For chunked storage, the dimensions define the size of a single chunk. They are in units of array elements (not bytes). The first dimension stored in the list of dimensions is the slowest changing dimension and the (n - 1) dimension stored is the fastest changing dimension.
For an n-dimensional dataset (for n >= 2), dimensions 0..(n-1) are stored and then the dataset element size is stored. The dataset element size is part of the dimensions "array", but it's not really a dimension itself.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Dimension 0 Size, Dimension 1 size,.... Dimension (n-1) size, Dataset element size"

Yes, this part is what I think the layout table should say.

For an n-dimensional dataset (for n >= 2), dimensions 0..(n-1) are stored and then the dataset element size is stored. The dataset element size is part of the dimensions "array", but it's not really a dimension itself.

These were just my own words summarizing the encoding; I think the text currently in the description table in this diff is fine as is and you don't need to add these words.

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Checklist

This PR touches the following areas. Each needs a sign-off
from its listed owners before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To be triaged

Development

Successfully merging this pull request may close these issues.

3 participants