fix: transfer GridEmbedding grid to input device (fixes CUDA scalar indexing #125)#132
Open
jitendravjh wants to merge 1 commit into
Open
Conversation
…ndexing SciML#125) GridEmbedding built the positional grid using CPU range/meshgrid, then called cat(grid, x) where x may be a CuArray. This caused: ERROR: Scalar indexing is disallowed. Invocation of getindex resulted in scalar indexing of a GPU array. Fix: call Lux.get_device(x)(grid) immediately after building the grid, so the array is moved to the same device as the input before the cat. This is a no-op on CPU and transparently transfers to GPU/Metal/etc. Fixes SciML#125
Author
|
The CI failures are pre-existing and unrelated to this change:
The same failures appear on |
Member
|
We'd need to get master working again to get merges here then. |
Author
|
I just opened a quick PR (#135) to fix the broken Windows tests on main (the Float32 finite difference tolerance was too strict for XLA on Windows). Once that's in, we should be unblocked! |
This file contains hidden or 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
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.
What this fixes
Calling
FourierNeuralOperatoron a GPU (CuArray) raised:Reported in #125.
Root cause
GridEmbeddingbuilds its positional coordinate grid from CPUrangevectors viameshgrid. The resultinggridis a plainArray{T}on the CPU. When:cat(grid, x; dims = N - 1)is called andxis aCuArray, Julia's fallbackcatpath reads from the GPU array element-by-element on the CPU — triggering the scalar indexing error.The second stacktrace in the issue points directly to the offending line:
Fix
After building the grid, move it to the same device as
xbefore thecat:Lux.get_deviceis already available via the existingLuximport and:CPUDevicefunctor that just callsArray(grid)which is already a no-opCUDADevicefunctor that callscu(grid)Checklist