Skip to content
This repository was archived by the owner on Jul 15, 2025. It is now read-only.

Fix bug when slicing on a segmented dimension#2

Merged
karllessard merged 1 commit intotensorflow:mainfrom
karllessard:segmented-fix
Jul 27, 2021
Merged

Fix bug when slicing on a segmented dimension#2
karllessard merged 1 commit intotensorflow:mainfrom
karllessard:segmented-fix

Conversation

@karllessard
Copy link
Contributor

This is a bug found by @JimClarke5 while working on the Sparse tensors. When slicing on a dimension that was segmented (i.e. which elements are no more sequential due to a previous slice), we were losing track of that information.

}
Dimension[] newDimensions = Arrays.copyOfRange(dimensions, dimensionStart, dimensions.length);
if (segmentationIdx > dimensionStart) {
if (segmentationIdx >= dimensionStart) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So the bug was repeated slicing from the start of the ndarray?

@JimClarke5
Copy link
Contributor

I tested it with my Sparse code and it now puts out the correct values in FloatSparseNdArrayTest.testSlice().
Amazing what a 1 character change can do 👍

Copy link
Contributor

@Craigacp Craigacp left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@JimClarke5 JimClarke5 left a comment

Choose a reason for hiding this comment

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

This bug also appears in the dense version, both dense and sparse now work for me with this change.

@karllessard karllessard merged commit 77aa450 into tensorflow:main Jul 27, 2021
@karllessard
Copy link
Contributor Author

Thanks @JimClarke5 and @Craigacp !

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants