Skip to content

Indexing: Use Param/Array::strides instead of toStride #2311

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

Merged
merged 1 commit into from
Oct 17, 2018

Conversation

mark-poscablo
Copy link
Contributor

This addresses #2273, and is a change of approach from the original PR #2303, since the problem doesn't lie in reorder itself, but in indexing (one of the tests in that PR was wrong too, which led me to believe that I was initially going the right direction there). This is a special case of reordering (or in general, modifying the dims/strides of) an array before indexing it, and more specifically using an af::array to index. For calculating the input offsets, indexing (on all backends) call src/backend/common/ArrayInfo.cpp:toStride, which calculates the input stride based on the seqs given, but the given af::array index is not an af::seq object, and thus produces the wrong input strides. To get the correct strides (the result of the Array::modStrides call within the internal reorder), I think that we should use the Param/Array::strides function instead.

Please feel free to suggest any better approaches that you might think of.

@mark-poscablo mark-poscablo changed the title Indexing: Use Param/Array::strides instead of toStrides Indexing: Use Param/Array::strides instead of toStride Oct 10, 2018
@9prady9
Copy link
Member

9prady9 commented Oct 11, 2018

Looks good to me.

umar456
umar456 previously approved these changes Oct 11, 2018
@9prady9 9prady9 merged commit 6dfd02f into arrayfire:master Oct 17, 2018
@mark-poscablo mark-poscablo deleted the fix-indexing branch October 29, 2018 17:50
@mlloreda mlloreda added this to the v3.6.2 milestone Nov 13, 2018
facebook-github-bot pushed a commit to flashlight/flashlight that referenced this pull request Apr 17, 2019
Summary:
When testing CUDA ForceAlign, I encountered NaNs that seemed unrelated to ForceAlign. After investigation I discovered the NaNs were introduced during GLU.

During one particular forward pass, in one particular GLU layer, the array

`input.array()(fhalf[0], fhalf[1], fhalf[2], fhalf[3])`

had NaNs, whereas `input.array()` did not have any NaNs. This seems to be due to an ArrayFire 3.6.1 bug:

- issue: arrayfire/arrayfire#2273
- PR: arrayfire/arrayfire#2311

This diff applies the `af::moddims` workaround I mentioned in the issue.

It does appear to be resolved in ArrayFire 3.6.2. But we can use this workaround until we update ArrayFire in TP2 to 3.6.2.

Reviewed By: vineelpratap

Differential Revision: D14961937

fbshipit-source-id: dcd455b82e7ce6888a2d35cba868718a55789f04
jacobkahn pushed a commit to flashlight/text that referenced this pull request Mar 25, 2022
Summary:
When testing CUDA ForceAlign, I encountered NaNs that seemed unrelated to ForceAlign. After investigation I discovered the NaNs were introduced during GLU.

During one particular forward pass, in one particular GLU layer, the array

`input.array()(fhalf[0], fhalf[1], fhalf[2], fhalf[3])`

had NaNs, whereas `input.array()` did not have any NaNs. This seems to be due to an ArrayFire 3.6.1 bug:

- issue: arrayfire/arrayfire#2273
- PR: arrayfire/arrayfire#2311

This diff applies the `af::moddims` workaround I mentioned in the issue.

It does appear to be resolved in ArrayFire 3.6.2. But we can use this workaround until we update ArrayFire in TP2 to 3.6.2.

Reviewed By: vineelpratap

Differential Revision: D14961937

fbshipit-source-id: dcd455b82e7ce6888a2d35cba868718a55789f04
jacobkahn pushed a commit to flashlight/text that referenced this pull request Mar 25, 2022
Summary:
When testing CUDA ForceAlign, I encountered NaNs that seemed unrelated to ForceAlign. After investigation I discovered the NaNs were introduced during GLU.

During one particular forward pass, in one particular GLU layer, the array

`input.array()(fhalf[0], fhalf[1], fhalf[2], fhalf[3])`

had NaNs, whereas `input.array()` did not have any NaNs. This seems to be due to an ArrayFire 3.6.1 bug:

- issue: arrayfire/arrayfire#2273
- PR: arrayfire/arrayfire#2311

This diff applies the `af::moddims` workaround I mentioned in the issue.

It does appear to be resolved in ArrayFire 3.6.2. But we can use this workaround until we update ArrayFire in TP2 to 3.6.2.

Reviewed By: vineelpratap

Differential Revision: D14961937

fbshipit-source-id: dcd455b82e7ce6888a2d35cba868718a55789f04
jacobkahn pushed a commit to flashlight/text that referenced this pull request Mar 25, 2022
Summary:
When testing CUDA ForceAlign, I encountered NaNs that seemed unrelated to ForceAlign. After investigation I discovered the NaNs were introduced during GLU.

During one particular forward pass, in one particular GLU layer, the array

`input.array()(fhalf[0], fhalf[1], fhalf[2], fhalf[3])`

had NaNs, whereas `input.array()` did not have any NaNs. This seems to be due to an ArrayFire 3.6.1 bug:

- issue: arrayfire/arrayfire#2273
- PR: arrayfire/arrayfire#2311

This diff applies the `af::moddims` workaround I mentioned in the issue.

It does appear to be resolved in ArrayFire 3.6.2. But we can use this workaround until we update ArrayFire in TP2 to 3.6.2.

Reviewed By: vineelpratap

Differential Revision: D14961937

fbshipit-source-id: dcd455b82e7ce6888a2d35cba868718a55789f04
jacobkahn pushed a commit to flashlight/sequence that referenced this pull request Oct 13, 2022
Summary:
When testing CUDA ForceAlign, I encountered NaNs that seemed unrelated to ForceAlign. After investigation I discovered the NaNs were introduced during GLU.

During one particular forward pass, in one particular GLU layer, the array

`input.array()(fhalf[0], fhalf[1], fhalf[2], fhalf[3])`

had NaNs, whereas `input.array()` did not have any NaNs. This seems to be due to an ArrayFire 3.6.1 bug:

- issue: arrayfire/arrayfire#2273
- PR: arrayfire/arrayfire#2311

This diff applies the `af::moddims` workaround I mentioned in the issue.

It does appear to be resolved in ArrayFire 3.6.2. But we can use this workaround until we update ArrayFire in TP2 to 3.6.2.

Reviewed By: vineelpratap

Differential Revision: D14961937

fbshipit-source-id: dcd455b82e7ce6888a2d35cba868718a55789f04
jacobkahn pushed a commit to flashlight/sequence that referenced this pull request Oct 13, 2022
Summary:
When testing CUDA ForceAlign, I encountered NaNs that seemed unrelated to ForceAlign. After investigation I discovered the NaNs were introduced during GLU.

During one particular forward pass, in one particular GLU layer, the array

`input.array()(fhalf[0], fhalf[1], fhalf[2], fhalf[3])`

had NaNs, whereas `input.array()` did not have any NaNs. This seems to be due to an ArrayFire 3.6.1 bug:

- issue: arrayfire/arrayfire#2273
- PR: arrayfire/arrayfire#2311

This diff applies the `af::moddims` workaround I mentioned in the issue.

It does appear to be resolved in ArrayFire 3.6.2. But we can use this workaround until we update ArrayFire in TP2 to 3.6.2.

Reviewed By: vineelpratap

Differential Revision: D14961937

fbshipit-source-id: dcd455b82e7ce6888a2d35cba868718a55789f04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants