BUG: CategoricalGibbsMetropolis doesn't respect the tune parameter

[Armavica]

Describe the issue:

I think that this PR changed the behaviour of the CategoricalGibbsMetropolis sampler which now treats all tune and draws as draws. By comparison, the behaviour of BinaryGibbsMetropolis was unchanged, so I am guessing that this change was unintended.

If intended I find this behaviour confusing and we should probably emit a warning message to clarify?

Reproduceable code example:

import pymc as pm
with pm.Model():
    pm.Categorical("cat", [0.02, 0.08, 0.9])
    idata = pm.sample(draws=20, tune=20)
# Sampling 4 chains for 0 tune and 40 draw iterations (0 + 160 draws total)
# Returns an idata object with 40 draws

Error message:

PyMC version information:

PyMC Version: 5.24 to current

Context for the issue:

No response

[ricardoV94]

Yeah. The sampler doesn't use tune so I thought it didn't matter. Also if pm.sample is the one controlling when tune of the samplers starts and ends why does it need to hear it back from the samplers?

[Armavica]

I understand the rationale but I think it does matter from a "least surprise" perspective: as it is, it is difficult for people who don't know the specifics of the samplers to understand why the tune parameter is not respected (a friend asked me for help after spending some time debugging this). I can see several options:

1. leave as it is (do 0 tune and tune+draws draws)
2. pretend the sampler is tunable, do the requested tuning even if that doesn't accomplish anything (do tune tune and draws draws)
3. skip the tuning, just do the draws (do 0 tune and draws draws)

For any of these options, I think we could additionally consider emitting a message explaining the behaviour.

I think I would go for 3. + message explaining that tuning is unnecessary for this sampler and skipped

[Jiya873]

Hi @Armavica @ricardoV94,
I agree this is confusing from a user perspective. I can try adding a warning when tune > 0 is passed to CategoricalGibbsMetropolis to explicitly state that tuning is skipped for this sampler.

[ricardoV94]

Thing is these samplers are usually used in a blocked setting with other samplers that do tune. Emitting a warning wouldn't make sense. Have fake tune draws is fine, my comment was just that pm.sample is the one deciding if a given draw is a tuning draw or not (even if the sampler doesn't need tuning or would like to do more tuning).

So it doesn't make sense to return a tune flag from each sampler, to decide what kind of draw it is. The sampler already knows it because it's the one deciding so.

[Jiya873]

I've analyzed the implementation of CategoricalGibbsMetropolis and compared it with BinaryGibbsMetropolis, and I think I’ve identified why this behavior occurs.

There seem to be two related reasons:

1. Unlike BinaryMetropolis and BinaryGibbsMetropolis, CategoricalGibbsMetropolis.__init__ does not accept or store a tune parameter. As a result, even when pm.sample is running a tuning phase, the sampler ignores it.

2. Both astep_unif and astep_prop explicitly return {"tune": False} in their stats, regardless of whether the iteration is part of tuning or sampling:

# Current behavior in astep_unif and astep_prop
return q, [{"tune": False}]

This effectively forces all iterations to be treated as draws, which explains the “0 tune, X draws” output seen in this issue.

One possible way to make this more consistent with BinaryGibbsMetropolis would be to:

• Allow CategoricalGibbsMetropolis to store a tune flag (even if it doesn’t actually tune any parameters), and
• Report {"tune": self.tune} in the sampler stats instead of always returning False.

This would keep the current non-adaptive behavior while aligning the reported tuning phase with pm.sample.

[ricardoV94]

If you read my last message you may come to a different conclusion...