Make Gibbs work with step_warmup

[mhauru]

Gibbs used to just ignore warm-up as a concept, and always called step rather than step_warmup. This was discussed in #2493. This PR makes Gibbs respect num_warmup by calling step_warmup on the component samplers whenever Gibbs itself is called with step_warmup. It also does the same thing for RepeatSampler. Note that this means that e.g. if you use RepeatSampler(spl, 3) and num_warmup=100 spl will take 300 warm-up steps.

#2493 should be left open for now, because we might want to improve this so that one could specify different amounts of warm-up for different component samplers. This is just a stop-gap bug fix to at least do the minimal sensible thing.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.88%. Comparing base (3cab967) to head (e2805a5).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2502      +/-   ##
==========================================
+ Coverage   84.59%   84.88%   +0.29%     
==========================================
  Files          21       21              
  Lines        1597     1628      +31     
==========================================
+ Hits         1351     1382      +31     
  Misses        246      246              

☔ 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 13704498769

Details

• 35 of 35 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+8.8%) to 84.889%

---

Totals
Change from base Build 13634287209:	8.8%
Covered Lines:	1382
Relevant Lines:	1628

---

💛 - Coveralls

[sunxd3]

Generally looks very good. One concern: from the coverage, repeat_sampler is not tested, is it worthwhile to add a test?

[sunxd3]

would this create some kind of type instability? my first order thought is that Function is abstract type

[mhauru]

I think it should be fine, since each function has its own type. E.g.

julia> typeof(identity)
typeof(identity) (singleton type of function identity, subtype of Function)

julia> typeof(sin)
typeof(sin) (singleton type of function sin, subtype of Function)

The ::Function bit just enforces that you can't pass as the step_function argument anything that isn't of type Function, but the compiler will still see the concrete type of the argument. I also checked that making this change didn't have a substantial impact on runtime.

[mhauru]

Hmm, I'm confused by the coverage complaint, I thought these lines from test/mcmc/gibbs.jl would have tested those lines:

    num_reps = 2
    wuc = WarmupCounter()
    sample(m, Gibbs(:x => RepeatSampler(wuc, num_reps)), num_samples; num_warmup=num_warmup)
    @test wuc.warmup_init_count == 1
    @test wuc.non_warmup_init_count == 0
    @test wuc.warmup_count == num_warmup * num_reps
    @test wuc.non_warmup_count == (num_samples - 1) * num_reps

Need to check this next week.