Updated Dropout for more input types.

[ShoofLLC]

Replacing the call to rand! in the _dropout_mask function to account
for complex (float) data types.

See discussion: #1572

PR Checklist

• Tests are added
• Entry in NEWS.md
• Documentation, if applicable

[codecov-commenter]

Codecov Report

Merging #1867 (ccb328c) into master (7b56813) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1867      +/-   ##
==========================================
+ Coverage   84.50%   84.51%   +0.01%     
==========================================
  Files          21       21              
  Lines        1484     1485       +1     
==========================================
+ Hits         1254     1255       +1     
  Misses        230      230              

Impacted Files	Coverage Δ
src/layers/normalise.jl	83.13% <100.00%> (+0.10%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b56813...ccb328c. Read the comment docs.

[ShoofLLC]

The return type of dropout if NOT "active" for Int types would be Int.
But when "active" it would be Float.

That doesn't seem like a good thing, so I think that's a no on this one too.

[darsnack]

Can you add an entry in NEWS.md?

[ShoofLLC]

Can you add an entry in NEWS.md?

This still needs a new review, I've found something while testing with int types.

[ToucheSir]

The return type of dropout if NOT "active" for Int types would be Int.
But when "active" it would be Float.

This behaviour is already present on master. I don't think it needs to be addressed in this PR. If anything, this PR improves compatibility with int inputs, as before dropout_mask was calling rand!(::AbstractArray{Int}) and silently doing the wrong thing...

[darsnack]

The fix should make the inactive path return a chosen float type. It doesn't make sense to return an integer with the scaling.

[ToucheSir]

The tricky part is that the inactive path doesn't require any scaling. Or put another way, it's semantically equivalent to p=0, which leads to a scaling factor 1/(1-p) === 1. I suppose we could specifically convert or error on integer arrays?

[darsnack]

True, but I think the return type should be stable across active/inactive regardless of the actual computation. 1 / (1 - 0) is a float, so I think always converting is reasonable here.

[ShoofLLC]

True, but I think the return type should be stable across active/inactive regardless of the actual computation. 1 / (1 - 0) is a float, so I think always converting is reasonable here.

The tricky part is that the inactive path doesn't require any scaling. Or put another way, it's semantically equivalent to p=0, which leads to a scaling factor 1/(1-p) === 1. I suppose we could specifically convert or error on integer arrays?

Since scaling is not optional, it kinda makes sense to error, it would at least let the user know that something is wrong.

[darsnack]

It's not exactly an error, right? Mathematically the correct output exists, but it cannot be an integer except for very limited values of p. Perhaps we can warn though cause converting the type might cause unexpected downstream errors.

[darsnack]

Either way, I agree with @ToucheSir that we should merge this and then deal with the type stability issue in a separate PR.

[darsnack]

Shall we merge this?