Repository navigation
Adding Tensor.roll and torch.roll #621 - #623
Conversation
NiklasGustafsson
changed the title
| /// Elements that are shifted beyond the last position are re-introduced at the first position. | ||
| /// If a dimension is not specified, the tensor will be flattened before rolling and then restored to the original shape. | ||
| /// </summary> | ||
| public Tensor roll((long,long) shifts, (long,long) dims) |
There was a problem hiding this comment.
I wasn't able to make the 'dims' argument a span, since it didn't let me default to 'null' Is there a solution to that? An empty span?
There was a problem hiding this comment.
You can just pass empty span when having null dims. You can pass default value for the dims parameter.
| /// Elements that are shifted beyond the last position are re-introduced at the first position. | ||
| /// If a dimension is not specified, the tensor will be flattened before rolling and then restored to the original shape. | ||
| /// </summary> | ||
| public Tensor roll((long,long) shifts, (long,long) dims) |
| /// Elements that are shifted beyond the last position are re-introduced at the first position. | ||
| /// If a dimension is not specified, the tensor will be flattened before rolling and then restored to the original shape. | ||
| /// </summary> | ||
| public Tensor roll(IEnumerable<long> shifts, IEnumerable<long>? dims = null) |
tarekgh
left a comment
tarekgh
left a comment
There was a problem hiding this comment.
Added minor comments and a suggestion if you want to consider it. LGTM, otherwise.
| public Tensor roll(long shifts, long? dims = null) | ||
| { | ||
| if (dims.HasValue) { | ||
| return roll(new long[] { shifts }, new long[] { dims.Value }); |
There was a problem hiding this comment.
Hmm... When I tried changing to ReadOnlySpan for a few other APIs (torch.zeros, for example), that seems to not work with F#.
@dsyme -- this doesn't work if torch.zeros takes a ReadOnlySpan:
let mutable pe = torch.zeros([| maxLen; dmodel|])Do I need to overload ROS and IEnumerable?
There was a problem hiding this comment.
Do I need to overload ROS and IEnumerable?
It seems I need to have roll(ReadOnlySpan,...) and roll(long[],...) as well as a private _roll(ReadOnlySpan...) with the actual implementation.
|
I've been looking for other places to use ReadOnlySpan, but I will put that into separate PR. |
Fixed issue #621