[am17an]

Overview

The ggml_set_output now also sets the src tensors of views as outputs as well. This is to prevent from the allocator from reusing their memory.

Requirements

• I have read and agree with the contributing guidelines
• AI usage disclosure: no

[ggerganov]

A bit more info would be helpful - where do we use views for output?

[am17an]

I'm adding the deepseek4 MTP, there we need to add a hidden state from a reshape (which is a view)

llama.cpp/src/models/deepseek4.cpp

Lines 1156 to 1160 in c3d47e6

	if (inp_out_ids) {
	ggml_tensor * flat = ggml_reshape_2d(ctx0, inpL, n_embd*hc, n_tokens);
	flat = ggml_get_rows(ctx0, flat, inp_out_ids);
	inpL = ggml_reshape_3d(ctx0, flat, n_embd, hc, n_outputs);
	}

[ggerganov]

I think you can simply output the hidden state before the reshape? The hidden state extraction logic only needs to know the total size of the tensor - the specific dimensions are not relevant.

[am17an]

The unflattened tensor is 3d, get_embeddings_nextn expects a 2-d tensor of dim [n_outputs][n_embd]

[am17an]

Also in general this is a bug right? If any t is a view then this can possibly cause issues. We have no documentation that says that these things can't be views.

[ggerganov]

Also in general this is a bug right? If any t is a view then this can possibly cause issues. We have no documentation that says that these things can't be views.

Yes, we should assert in the set_outputs() that none of the outputs are views.

The unflattened tensor is 3d, get_embeddings_nextn expects a 2-d tensor of dim [n_outputs][n_embd]

I don't follow. Is this a problem? Normally, all embeddings and hidden states that we extract are 2D.

[am17an]

It's not a problem for the MTP, I'll just update this PR to add an assert for outputs not being views then

[ggerganov]

Some server tests are failing after this change: https://github.com/ggml-org/llama.cpp/actions/runs/29490535879/job/87595298014?pr=25729#step:7:752

[am17an]

It looks like some cases with backend_sampling: true are failing

[am17an]

This is the problem. The top-p default returns a view as output

llama.cpp/src/llama-sampler.cpp

Lines 1434 to 1458 in fd4fd67

	auto ggml_sort = [ctx](struct ggml_tensor * a, struct ggml_tensor * b) {
	GGML_ASSERT(ggml_nrows(a) == 1);
	struct ggml_tensor * a_reshaped = ggml_reshape_2d(ctx, a, 1, a->ne[0]);
	struct ggml_tensor * a_sorted = ggml_get_rows(ctx, a_reshaped, b);
	return ggml_reshape_1d(ctx, a_sorted, a->ne[0]);
	};
	// Get the sorted logits in descending order.
	struct ggml_tensor * sorted_idx = ggml_argsort(ctx, data->logits, GGML_SORT_ORDER_DESC);
	ggml_set_name(sorted_idx, "top_p_sorted_idx");
	// Do the sorting via reshape + get_rows
	struct ggml_tensor * sorted_logits = ggml_sort(data->logits, sorted_idx);
	ggml_set_name(sorted_logits, "top_p_sorted_logits");
	struct ggml_tensor * softmax = ggml_soft_max(ctx, sorted_logits);
	ggml_set_name(softmax, "top_p_softmax");
	// If candidates are provided, sort them as well. Otherwise, set sorted indices as candidates.
	if (data->candidates) {
	data->candidates = ggml_sort(data->candidates, sorted_idx);
	} else {
	data->candidates = sorted_idx;
	}
	ggml_set_name(data->candidates, "top_p_candidates");

[ggerganov]

Should be good to merge. Might have some impact on the #25532

[gaugarg-nv]

I faced a similar issue in #25532

How about fixing it within GGML?

We can propagate the output flag to the backing tensor in ggml_set_output. Something like this-

void ggml_set_output (struct ggml_tensor * tensor) {
  for (ggml_tensor * cur = tensor; cur != nullptr; cur = cur->view_src) {
      cur->flags |= GGML_TENSOR_FLAG_OUTPUT;
  }
}

[ggerganov]

I was a bit hesitant to make it since I initially thought that we don't need to use output views. But it does seem that there are some use cases where it's complicated to avoid having a view for output.

[gaugarg-nv]

I see that @am17an's first commit in this PR did something similar. But it's not clear to me why an output can't be a "view".
As long as we make sure the backing tensor stays alive during graph execution, we should be fine.

The current assert also enforces this only for tensors registered in llm_graph_result. What if ggml_set_output is called on a view somewhere else?

[ggerganov]

Ok, I added the change to ggml_set_output. Will keep the changes to the llama-sampler.cpp since I think they improve consistency and should resolve some edge-cases when shaping the input logits of the samplers.