Skip to content

Conversation

@michaelgsharp
Copy link
Contributor

Fixes #106539, #106537, and #106535.

They all had to go in one PR as they were all connected, but basically fixes Reshape/SetSlice to work with 0 strides. Also adds a few more checks to make sure its ok to Reshape the values.

@soerenwolfers
Copy link

With this proposal, the definition of Reshape would be sufficiently restrictive that I think

  • the documentation should be extended to specify in more detail what "the new shape is not compatible with the old shape" means
  • the functionality should be hidden behind a TryReshape method.

Also, note that the bit where numpy and the current TensorSpan deviate in #106539 is just a.Reshape(a.Lengths), on which numpy returns a whereas the proposal here would throw an exception, which it should probably not (although maybe there is a reasonable definition of Reshape that doesn't allow it and I just don't see it, see the first bullet point).

@michaelgsharp
Copy link
Contributor Author

@soerenwolfers I am about to push a change that will let a.Reshape(a.Lengths) always work.

We will always have some difference between numpy because numpy will allocate new memory behind the scenes if it needs to and we cannot allocate new memory for the TensorSpan. It should work just fine for Tensor though as most operations copy for that in comparison to the TensorSpan where its usually not a copy, so we shouldn't get into the memory situations where a reshape wouldn't work correctly.

I agree a TryReshape would be a good thing to have for TensorSpan but not for Tensor. @tannergooding any thoughts on any of this?

@michaelgsharp
Copy link
Contributor Author

/azp run runtime-dev-innerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@michaelgsharp
Copy link
Contributor Author

/backport to release/9.0

@github-actions
Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/11052885500

@michaelgsharp
Copy link
Contributor Author

@microsoft-github-policy-service rerun

@ericstj
Copy link
Member

ericstj commented Sep 26, 2024

@dotnet-policy-service rerun

@michaelgsharp michaelgsharp merged commit 735a031 into dotnet:main Sep 26, 2024
@michaelgsharp michaelgsharp deleted the set-slice-safety branch September 26, 2024 18:12
sirntar pushed a commit to sirntar/runtime that referenced this pull request Sep 30, 2024
* working

* comments from PR

* can always reshape to self

* fixed tests

* comments from PR

* fixing tests
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.Numerics.Tensors Should omit .Reshape function.

4 participants