[V1] Entrypoints Test - Enable

[robertgshaw2-redhat]

SUMMARY:

• enable tests

[github-actions]

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

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 either: Add ready label to the PR or enable auto-merge.

🚀

[russellb]

@aarnphm @njhill FYI -- This is where I finally landed that makes our tests pass for both Qwen and Mistral

[russellb]

but I'm definitely not confident in this ... I don't have a complete enough understanding of the factors involved here to know we're always getting the right value ...

[robertgshaw2-redhat]

Do we need the +1?

[russellb]

in the case that broke for me, get_vocab_size() returned the same as "max_token_id + 1".

There is another case, tested in CI, that needed the value "max_token_id" to work.

I'll probably run out of time today before I can get to the bottom of this properly...

[aarnphm]

hmm, i think we should use len(tokenizer.get_vocab()) to calculate it eagerly

#14851

[russellb]

I rebased on that and it’s still failing

[robertgshaw2-redhat]

Is the bitmask applied to the logits?

[russellb]

correct, applied here:

vllm/vllm/v1/worker/gpu_model_runner.py

Lines 869 to 914 in 270a5da

	def apply_grammar_bitmask(
	self,
	scheduler_output: "SchedulerOutput",
	logits: torch.Tensor,
	):
	# Serialization of np.ndarray is much more efficient than a tensor,
	# so we receive it in that format.
	grammar_bitmask = scheduler_output.grammar_bitmask
	if grammar_bitmask is None:
	return
	# We receive the structured output bitmask from the scheduler, but the
	# indices of the requests in the batch may not match the indices of
	# the bitmask since the scheduler doesn't know how the gpu runner is
	# ordering the requests in the batch. We need to sort the bitmask to
	# match the order of the requests used here.
	struct_out_req_batch_indices: dict[str, int] = {}
	indices_match = True
	for req_id in self.input_batch.req_ids:
	mask_index = scheduler_output.structured_output_request_ids.get(
	req_id)
	if mask_index is None:
	# not a structured output request
	continue
	batch_index = self.input_batch.req_id_to_index[req_id]
	if batch_index != mask_index:
	indices_match = False
	struct_out_req_batch_indices[req_id] = batch_index
	if not indices_match:
	# Sort the bitmask to match the order of the requests
	sorted_bitmask = np.zeros_like(grammar_bitmask)
	for req_id, batch_index in struct_out_req_batch_indices.items():
	orig_index = scheduler_output.structured_output_request_ids[
	req_id]
	sorted_bitmask[batch_index] = grammar_bitmask[orig_index]
	grammar_bitmask = sorted_bitmask
	grammar_bitmask = torch.from_numpy(grammar_bitmask)
	# TODO: compatibility with spec decode
	xgr.apply_token_bitmask_inplace(
	logits,
	grammar_bitmask.to(self.device, non_blocking=True),
	indices=list(struct_out_req_batch_indices.values()),
	)

[robertgshaw2-redhat]

It would make sense to me that this is the model vocab_size then, since then it matches the shape of the logits

[mergify]

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

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

[russellb]

The tests passed in CI, though the change is very suspicious. It could use more analysis and a proper explanation of the right thing, but it's definitely better than it was ...

I can try to dig into this for a proper explanation (and likely another change) to put this to bed for good next week

[russellb]

I rebased this on main. I THINK that all fixes necessary are in main and we're down to the one line that turns on the tests ... let's see what CI says

[russellb]

Looks like @aarnphm ’s last fix for the xgrammar bitmask didn’t do the trick.

[russellb]

replaced by #14903