Skip to content

Misc. bug: new kv cell seq implementation does not handle "seq_id = -1" specified in the API #13983

@l3utterfly

Description

@l3utterfly

Name and Version

The API in llama.h specifies:

// Removes all tokens that belong to the specified sequence and have positions in [p0, p1)
    // Returns false if a partial sequence cannot be removed. Removing a whole sequence never fails
    // seq_id < 0 : match any sequence
    // p0 < 0     : [0,  p1]
    // p1 < 0     : [p0, inf)
    LLAMA_API bool llama_kv_self_seq_rm(
            struct llama_context * ctx,
                    llama_seq_id   seq_id,
                       llama_pos   p0,
                       llama_pos   p1);

This doesn't seem to work anymore with the new kv cell implementation. In particular in the llama-kv-cells.h, there are assertions for seq_id >= 0, e.g.:

// check if the cell contains seq_id
    bool seq_has(uint32_t i, llama_seq_id seq_id) const {
        assert(i < pos.size());
        assert(seq_id >= 0);

        return seq[i].test(seq_id);
    }

Are we planning to keep the "any sequence" feature in the api?

Note, the seq = -1 input is still being used in main, when loading session caches, so it currently does not work. After loading session, it does this:

// remove any "future" tokens that we might have inherited from the previous session
        llama_kv_self_seq_rm(ctx, -1, n_matching_session_tokens, -1);

Operating systems

No response

Which llama.cpp modules do you know to be affected?

No response

Command line

Problem description & steps to reproduce

  1. build main project
  2. run, save session, then load session

First Bad Commit

No response

Relevant log output

bitset test argument out of range

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions