Call Tensor::alias() when returning an input Tensor

[ds5678]

This ensures that disposing the result has the same impact as it would with a different size.

[ds5678]

The continuous integration failed, but I don't see how I did anything wrong.

[yueyinqiu]

TestLoadJIT_3 sometimes just fail. That's not your problem.

By the way, you could edit RELEASENOTES.md as well. (I don't know, since we didn't do that in #1421.)

[ds5678]

By the way, you could edit RELEASENOTES.md as well. (I don't know, since we didn't do that in #1421.)

I haven't done this because the wording was ambiguous on whether or not I should, and I didn't want to cause merge conflicts with any other pull requests.

[ds5678]

Is there anything I need to do in order for this to be merged? It's blocking my third pull request.

[alinpahontu2912]

Hey @ds5678, I rebased your branch, if tests pass, can you also add an explanation in releasenotes ? We can then merge the pr :)

[ds5678]

Thanks for the review!

[ozanMSFT]

@ds5678 , thanks for your input and the PR.

Overall your change seems ok however I'm trying to understand the justification behind this code change.

This ensures that disposing the result has the same impact as it would with a different size

Could you provide further clarification in the description or perhaps a simple, reproducible code example with a brief before-and-after explanation? For instance, what was the issue, and how does this change resolve it?

[ds5678]

My pull request is based on the opinionated stance that disposing the tensor output from any function should never affect the inputs to that function.

I originally encountered this issue while concatenating tokens into a sequence during auto regression. I use the using Tensor approach to memory management because I find it to be the most predictable and performant. The only footgun I encountered with this approach was:

using Tensor sequence = torch.concat(tokens);

It disposed my first token (when my token list had count 1). Then, I auto regressed, so the list had count 2. It throws an error on the second pass because you can't concatenate a disposed tensor.

[ds5678]

Here's another example:

public override Tensor forward(IReadOnlyList<CilInstructionData> instructions)
{
	Tensor[] rentedArray = ArrayPool<Tensor>.Shared.Rent(instructions.Count);
	ArraySegment<Tensor> tensors = new(rentedArray, 0, instructions.Count);

	for (int i = 0; i < instructions.Count; i++)
	{
		tensors[i] = tokenAssembler.forward(instructions[i]);
	}

	Tensor result = concat(tensors);

	tensors.Dispose(); // extension method that iterates over the segment disposing each element
	ArrayPool<Tensor>.Shared.Return(rentedArray, true);

	return result; // If "instructions" has length 1, this will have already been disposed.
}

[ds5678]

Is there anything else I need to do?

[ozanMSFT]

LGTM with the additional explanation

[ozanMSFT]

@ds5678 , thanks for the contribution and sorry for the late reply.

It is always helpful to see the example and justification.