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

add _rng_compat_array #458

Merged
merged 2 commits into from
Jan 7, 2023
Merged

add _rng_compat_array #458

merged 2 commits into from
Jan 7, 2023

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Jan 7, 2023

No description provided.

src/dropout.jl Outdated Show resolved Hide resolved
Co-authored-by: Brian Chen <[email protected]>
@mcabbott mcabbott merged commit 4832bb8 into FluxML:master Jan 7, 2023
@mcabbott mcabbott deleted the dropout2 branch January 7, 2023 16:59
@@ -62,6 +63,7 @@ dropout!(B::AbstractArray, A::AbstractArray, p::Real; dims = :) = dropout!(_rng_
function dropout!(rng::AbstractRNG, dst::AbstractArray, src::AbstractArray, p::Real; dims=:)
size(dst) == size(src) || throw(DimensionMismatch("dropout! expects output array the same size as input"))
0 <= p <= 1 || throw(ArgumentError("dropout expects a probability 0 <= p <= 1"))
_rng_compat_array(rng, A)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_rng_compat_array(rng, A)
_rng_compat_array(rng, src)

It also occurs to me that the docstring is not in line with the actual function signature. Any reason to use A/B instead of dst/src for the former?

Copy link
Member Author

Choose a reason for hiding this comment

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

No real reason, just ended up this way.

@ToucheSir
Copy link
Member

Sorry, hit the review trigger a little early. Needs some typo fixes to make CI happy.

@mcabbott
Copy link
Member Author

mcabbott commented Jan 7, 2023

Done the hacky way in 9425e50

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.

3 participants