Enable mypy checking on V1 code

[markmc]

As per #10959

Some of these definitely deserve closer scrutiny e.g.:

• commit 7974f1d - was there a bug with the hash_block_tokens() call?
• commit 226fd55 - feels we'll be fighting an uphill battle with mypy and this args/kwargs pattern
• commit 8a6d88c - MultiprocExecutor.workers and UniprocExecutor.worker can be set to None, so we have to litter the code with is not None assertions

[github-actions]

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can do one of these:

• Add ready label to the PR
• Enable auto-merge.

🚀

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @markmc.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[markmc]

@DarkLight1337 @WoosukKwon note that I've rebased and fixed a bunch of new issues. Those new fixes are in the 5 latest commits

[markmc]

rebased again to fix conflicts

@comaniac would you mind reviewing commit 9190f34 ?

For reference, in #9972 you said:

Enhance the block hash type from int to Tuple[int, Tuple[int]], which is (hash_value, (toke_ids,)). This guarantees no hash conflicts

Pasting the commit message:

The current block hash format is:

  (hash((parent_hash, *token_id_list)), token_id_list)

i.e. it is a tuple with two parts:

1. A hash of a tuple combining the parent hash with token ids
2. A tuple containing the same token ids

and this (hash, token_ids) tuple itself must be hashable in order to be a dictionary key.

This is suspicious - why include the same token IDs twice in the same hash?

And then this code:

def hash_request_tokens(block_size: int,
                        token_ids: List[int]) -> List[BlockHashType]:
    ...
    ret = []
    parent_block_hash = None
    for ...:
        tuple(token_ids[start:end])
        ...
        block_hash = hash_block_tokens(parent_block_hash, block_token_ids)
        parent_block_hash = block_hash
    ...

looks wrong because hash_block_tokens() expects an integer hash for the parent_block_hash argument:

def hash_block_tokens(parent_block_hash: Optional[int],
                      curr_block_token_ids: Tuple[int]) -> BlockHashType:

It appears like it would be perfectly sufficient to use the integer hash of the (parent_hash + token_ids) tuple.

If we do this, then the curr_block_token_ids argument no longer needs to be a tuple - but it does need to be a sliceable sequence, so let's make sure ConstantList satisfies that requirement.

[comaniac]

This is suspicious - why include the same token IDs twice in the same hash?

This is to avoid potential hash collision, because Python dict guarantees no collision if the dict key is tuple.
But yeah the type is incorrect as you said. I'll submit a fix PR in minutes.

[markmc]

This is suspicious - why include the same token IDs twice in the same hash?

This is to avoid potential hash collision, because Python dict guarantees no collision if the dict key is tuple. But yeah the type is incorrect as you said. I'll submit a fix PR in minutes.

Thanks for the fix! Rebased again to resolve merge conflicts and fixed a few mypy errors with this prefix caching code.

[markmc]

Moving to draft while I debug the test failures

[WoosukKwon]

@markmc Finally merged! Thanks for your patience and great work! Really appreciate it.