Simplify Embedding

[mcabbott]

Embedding has some special code for OneHotMatrix which (1) will break with latest changes, and (2) doesn't allow higher-rank arrays the way that "index" input does:

julia> Embedding(26 => 10)(rand(1:26)) |> size
(10,)

julia> Embedding(26 => 10)(rand(1:26, 2,3,4,5)) |> size
(10, 2, 3, 4, 5)

julia> Embedding(26 => 10)(onehot(rand(1:26), 1:26)) |> size
(10,)

julia> Embedding(26 => 10)(onehotbatch(rand(1:26, 2), 1:26)) |> size  # can't go further
(10, 2)

So this PR simplifies & adds reshape.

I did this after forgetting that #1656 exists, some overlap. This PR does not attempt to fix outputsize. Some other changes there have already happened elsewhere.

[mcabbott]

These could instead call Flux.onecold. The result will differ on e.g. [true, true, false], not sure we care too much either way?

[darsnack]

For performance in the one hot case? If it's onecold-compatible, then folks should use OneHotArray for performance. At least with *, we do the mathematically expected operation.

[mcabbott]

For OneHotArray these should be identical, right? Result and performance.

For a one-hot BitArray, the results will agree. I would guess that onecold is faster but haven't checked.

For a generic BitArray, I'm not sure which is mathematically expected really. I think you're saying that * is.

[darsnack]

Yes, what you wrote is what I meant re: performance. I was adding that in the one-hot bit array case, we can direct people to OneHotArray if their concern is performance.

Yeah whenever I've come across this type of operation in papers, I see it written as *. There's an implicit assumption that x is one-hot, so maybe onecold could be better here if it were made to error for [true, true, false], etc. But I think silently choosing the first "hot" index is wrong.

[mcabbott]

Yes. Mixing two embedding vectors seems less wrong. But probably nobody ever hits this & it's just a way to decouple from OneHotArray types. I don't think we should document that boolean indexing is an option.

[darsnack]

So I think we are happy with the current implementation in the PR?

[mcabbott]

Yes I think so.

I see we had a very similar discussion in #1656 (comment) BTW, I forgot... but same conclusion.

[darsnack]

Looks ready to me. Is there more you wanted to do here?

[darsnack]

So I think we are happy with the current implementation in the PR?

[mcabbott]

The "more" is #2088, really. Will merge when green.