Do not take an initial step before starting the chain in HMC

[penelopeysm]

Closes #2673

[github-actions]

Turing.jl documentation for PR #2674 is available at:
https://TuringLang.github.io/Turing.jl/previews/PR2674/

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.79%. Comparing base (296f654) to head (97ce752).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2674      +/-   ##
==========================================
- Coverage   85.86%   85.79%   -0.07%     
==========================================
  Files          22       22              
  Lines        1429     1422       -7     
==========================================
- Hits         1227     1220       -7     
  Misses        202      202              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coveralls]

Pull Request Test Coverage Report for Build 17642662193

Details

• 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.08%) to 83.639%

---

Totals
Change from base Build 17557004860:	-0.08%
Covered Lines:	1186
Relevant Lines:	1418

---

💛 - Coveralls

[sunxd3]

In general, no objection. The failed tests also look unrelated, so good.

On versioning, do you think this constitute a breaking change? Not saying I do, but worth second thought.

[penelopeysm]

I think generally for random results we shouldn't guarantee the exact numerical results as being part of the API, but we should guarantee some statistical properties. Since this doesn't change convergence properties for MCMC I think it's fine to put as a bugfix. I guess this isn't an official policy though so maybe worth checking if we all agree?

[sunxd3]

My concern is the behavior of initialization. Before this PR, the initialization wasn't returned in the chain.

[penelopeysm]

There is actually a subtle change that I just realised with this: when the initial state is included, then there are no sampler stats for the first entry in the chain (because the sampler hasn't run). That means they all get a value of missing.

Because MCMCChains requires a consistent eltype for all parameters + stats, this means that all the parameters are stored as Union{Float64,Missing}::

julia> using Turing; @model f() = x ~ Normal(); c = sample(f(), HMC(0.1, 5), 100)
Sampling 100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████| Time: 0:00:00
Chains MCMC chain (100×13×1 reshape(::Matrix{Union{Missing, Float64}}, 100, 13, 1) with eltype Union{Missing, Float64}):

Iterations        = 1:1:100
Number of chains  = 1
Samples per chain = 100
Wall duration     = 0.26 seconds
Compute duration  = 0.26 seconds
parameters        = x
internals         = hamiltonian_energy, n_steps, numerical_error, loglikelihood, hamiltonian_energy_error, is_accept, logprior, log_density, step_size, acceptance_rate, lp, nom_step_size

Use `describe(chains)` for summary statistics and quantiles.

julia> eltype(c[:x])
Union{Missing, Float64}

On main, eltype(c[:x]) would just return Float64.

This to me is not actually a Turing problem. It's an MCMCChains problem. But I wonder if that would factor into the decision.

[penelopeysm]

i'll change the base to breaking and merge