Error if init_params has not been specified for all chains

[torfjelde]

It seems we don't error correctly for certain multi-chain sample calls; in particular, Serial and MCMCDistributed seem to have this issue

AbstractMCMC.jl/src/sample.jl

Lines 522 to 526 in d7c549f

	chains = if init_params === nothing
	map(sample_chain, 1:nchains, seeds)
	else
	map(sample_chain, 1:nchains, seeds, init_params)
	end

AbstractMCMC.jl/src/sample.jl

Lines 469 to 473 in d7c549f

	chains = if init_params === nothing
	Distributed.pmap(sample_chain, pool, seeds)
	else
	Distributed.pmap(sample_chain, pool, seeds, init_params)
	end

while MCMCThreads does not, as it makes of _first_or_nothing:

https://github.com/TuringLang/AbstractMCMC.jl/blob/d7c549fe41a80c1f164423c7ac458425535f624b/src/sample.jl#L315C21-L316

AbstractMCMC.jl/src/sample.jl

Lines 542 to 549 in d7c549f

	function _first_or_nothing(x, n::Int)
	y = _first(x, n)
	length(y) == n || throw(
	ArgumentError("not enough initial parameters (expected $n, received $(length(y))"),
	)
	return y
	end
	_first_or_nothing(::Nothing, ::Int) = nothing

Doesn't seem like there's a reason why we don't use _first_or_nothing for MCMCSerial and MCMCDistributed; guessing it's just an oversight?

Ref: TuringLang/Turing.jl#2079

[torfjelde]

Note that the check in _first_or_nothing is not sufficient to catch scenarios where length(y) == n just happens to hold because the number of chains is the same as the dimension of the parameters 😬 But we can't really catch this here given that elements of init_params could be anything (even if init_params[1] is a Float64, it could be that the inner sample call expects Float64 as the init_params argument)

[devmotion]

Doesn't seem like there's a reason why we don't use _first_or_nothing for MCMCSerial and MCMCDistributed

Can you explain why we should use it? In those two cases we call map/pmap which require argument of matching length already, so it should not be possible to use init_params of incorrect length. We also can't call these functions with nothing, so we have to check for nothing explicitly anyway and then call the method without additional argument.

[torfjelde]

we call map/pmap which require argument of matching length already

It doesn't 😬 map will simply just use the shortest inputs:

julia> map(+, 1:2, 3:5)
2-element Vector{Int64}:
 4
 6

Alternatively we can use zip or something, which does require same length inputs

[devmotion]

Oops, I'm not sure why but I was convinced that map and pmap demand arguments of the same length but you're right. (BTW zip does also not require the arguments to be of the same length - I was aware of this but I always assumed this was an exception.)

[torfjelde]

(BTW zip does also not require the arguments to be of the same length - I was aware of this but I always assumed this was an exception.)

Oh true, haha. I had the exact opposite impression of what you did: I knew map didn't demand this, but I thought zip did 🤦