Skip to content
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

Avoiding scalar indexing in cuda #313

Merged
merged 2 commits into from
Apr 15, 2021
Merged

Conversation

yuehhua
Copy link
Member

@yuehhua yuehhua commented Apr 14, 2021

Existing implementation use scalar indexing for cuarray. To avoid using scalar indexing, refactoring is done.

@yuehhua yuehhua requested a review from CarloLucibello April 14, 2021 16:56
@ToucheSir
Copy link
Member

The CI failure appears to be unrelated to this change? AFAICT none of the conv machinery uses maximum_dims.

@yuehhua
Copy link
Member Author

yuehhua commented Apr 14, 2021

maximum_dims is used in scatter operations in scatter.jl. Then, I am sure that the CI failure is unrelated to this change.

src/utils.jl Outdated Show resolved Hide resolved
@DhairyaLGandhi
Copy link
Member

DhairyaLGandhi commented Apr 14, 2021

Mostly looks good.
Why not use maximum(dims)?

@DhairyaLGandhi
Copy link
Member

Reopening to see what's up with CI

@DhairyaLGandhi
Copy link
Member

Yeah, seems CI is unrelated. @yuehhua happy to merge if you can address replacing this with maximum

@yuehhua
Copy link
Member Author

yuehhua commented Apr 15, 2021

The functionality of this function is to calculate the maximum values across indices. For example, I have a array of tuples that contains a series of indices.

julia> A = [(5, 2), (3, 4), (1, 6)];

julia> maximum_dims(dims::AbstractArray{NTuple{N, T}}) where {N,T} = Tuple(maximum(x->x[i], dims) for i = 1:N);

julia> maximum_dims(A)
(5, 6)

julia> maximum(A)
(5, 2)

The maximum value of first component across indices is 5 and the maximum value of second component is 6. So, I want maximum_dims to return me the tuple (5, 6), while maximum give me the element which has maximum value in its first component.

src/utils.jl Outdated Show resolved Hide resolved
Co-authored-by: Carlo Lucibello <carlo.lucibello@gmail.com>
@DhairyaLGandhi
Copy link
Member

Thanks all

@DhairyaLGandhi DhairyaLGandhi merged commit c30ea9b into FluxML:master Apr 15, 2021
@yuehhua yuehhua deleted the refactor branch April 15, 2021 07:47
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