Skip to content

Conversation

@CarloLucibello
Copy link
Member

@CarloLucibello CarloLucibello commented May 14, 2021

fix #971

I could not fix the gradient + Dict case, if someone has any idea on how to that it would be appreciated, otherwise will just leave the broken test there

@darsnack
Copy link
Member

Does

Base.issetequal(x::Params, y::Base.KeySet) = issetequal(x.params, y)

work? What was the error with AbstractSet (may not want to define it so broadly anyways)?

@CarloLucibello
Copy link
Member Author

What was the error with AbstractSet (may not want to define it so broadly anyways)?

turns AbstractSet works indeed, the problem was in the dictionary creation, this gives an error

w = cu(rand(2))
Dict(w => w)

CuArrays are not hashable and IdDict should be used instead.

I currently overload issetequal(::Params, ::AbstractSet) for generality, but I'm never sure when those kind of general method overloads would lead to ambiguities in the wild. Better go with the stricter Base.KeySet? Although that seems too limited, I think I prefer AbstractSet

@CarloLucibello
Copy link
Member Author

CarloLucibello commented May 15, 2021

Nightly fail unrelated and fixed in FluxML/IRTools.jl#86

Copy link
Member

@darsnack darsnack left a comment

Choose a reason for hiding this comment

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

LGTM!

@CarloLucibello
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented May 16, 2021

Build succeeded:

@bors bors bot merged commit b59d30f into master May 16, 2021
@bors bors bot deleted the cl/setequal branch May 16, 2021 20:01
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.

Grads algebra on GPU causes scalar getindex is disallowed error

4 participants