improvements to stack

[CarloLucibello]

Fix #121 and add a rrule for stack.

Edit:
Remove the stack implementation in favor of the one from Base. The change is technically breaking since we don't support anymore dims > N+1 (see #119 (comment))

Fix #121 fix #119

[codecov-commenter]

Codecov Report

Merging #125 (531174a) into main (125205b) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #125      +/-   ##
==========================================
- Coverage   88.75%   88.61%   -0.14%     
==========================================
  Files          13       13              
  Lines         578      571       -7     
==========================================
- Hits          513      506       -7     
  Misses         65       65              

Impacted Files	Coverage Δ
src/utils.jl	90.40% <100.00%> (-0.51%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[darsnack]

Errors due to #119

[CarloLucibello]

I think it is better to wait for #124 to be merged and then remove the stack implementation from here

[darsnack]

Based on the discussion in #119, it doesn't seem like Base.stack/Compat.stack are 1-1 replacements for MLUtils.stack. I'm mostly concerned about MLUtils.stack(x, 2) style conflicting with Compat.stack(f, xs).

I think it is better to wait for #124 to be merged

You mean so we release both changes together? Or is there a connection to Tables.jl that I am missing?

[CarloLucibello]

Based on the discussion in #119, it doesn't seem like Base.stack/Compat.stack are 1-1 replacements for MLUtils.stack. I'm mostly concerned about MLUtils.stack(x, 2) style conflicting with Compat.stack(f, xs).

MLUtils.stack(x, 2) was deprecated already with v0.2 in favour of the keyword arg version, so we should be fine in removing it for v0.3.

I think it is better to wait for #124 to be merged

You mean so we release both changes together? Or is there a connection to Tables.jl that I am missing?

ops, I mentioned the wrong PR, I meant JuliaDiff/ChainRules.jl#681

[mcabbott]

This method should also call stack (if it cannot be removed):

MLUtils.jl/src/utils.jl

Lines 332 to 342 in 855f95b

	function batch(xs::AbstractArray{<:AbstractArray})
	# Don't use stack(xs, dims=N+1), it is much slower.
	# Here we do reduce(vcat, xs) along with some reshapes.
	szxs = size(xs)
	@assert length(xs) > 0 "Minimum batch size is 1."
	szx = size(xs[1])
	@assert all(x -> size(x) == szx, xs) "All arrays must be of the same size."
	vxs = vec(vec.(xs))
	y = reduce(vcat, vxs)
	return reshape(y, szx..., szxs...)
	end

[mcabbott]

Can simplify, as this will get Base version when available:

Suggested change

	if VERSION < v"1.9.0-DEV.1163"
	import Compat: stack
	else
	import Base: stack
	end
	using Compat: stack

[mcabbott]

Should probably bound Compat = "4.2". (Can't bound ChainRules but that's ok.)