Skip to content

reseed_chain() on a greedy chain removes the greedy sampler and adds dist — deterministic chain silently becomes stochastic #33

Description

@lloyal-research

sampler::create_chain() ends the chain differently depending on temperature:

  • temp > 0 → … → tempdist(seed)
  • temp ≤ 0 → … → greedy (no dist)

sampler::reseed_chain() assumes the first shape:

// Remove last sampler (dist by convention)
llama_sampler* old_dist = llama_sampler_chain_remove(chain, n - 1);
...
llama_sampler_chain_add(chain, llama_sampler_init_dist(new_seed));

Called on a greedy chain, this removes the greedy sampler and appends dist — converting a deliberately deterministic chain into a stochastic one, silently. No error, no log; the chain shape simply changes.

BranchStore tracks sampler_has_dist (set from temperature > 0 at creation) precisely so its callers can avoid this — but the guard lives in the caller's flag, not in the function that has the hazard. Any direct user of reseed_chain, or any future call site that forgets the flag, corrupts the chain.

Smallest fix

Make reseed_chain self-guarding: check whether the last sampler is dist (or accept a has_dist parameter) and no-op — or log loudly — on a greedy chain. The flag on BranchStore can stay as an optimisation; correctness shouldn't depend on it.

Found during a full read of sampler.hpp/branch.hpp; one of five silent-failure edges filed together.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions