Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Grads needed #3

Closed
6 of 8 tasks
alexbw opened this issue Oct 12, 2015 · 13 comments
Closed
6 of 8 tasks

Grads needed #3

alexbw opened this issue Oct 12, 2015 · 13 comments

Comments

@alexbw
Copy link
Collaborator

alexbw commented Oct 12, 2015

  • repeatTensor
  • max
  • min
  • cat
  • new
  • fill
  • index
  • __index (operator b = a[...])
@clementfarabet
Copy link
Collaborator

note: torch.cat implemented but only with 2 inputs (table not supported – table support was added to the public branch recently)

@alexbw
Copy link
Collaborator Author

alexbw commented Oct 19, 2015

Shouldn't need fill new and copy, since those produce tensors that, if used in computation, will be wrapped in nodes. If not used, and sent as output, then output will be independent of input.

@nkoumchatzky
Copy link
Contributor

What do you mean by newindex here?

@clementfarabet
Copy link
Collaborator

Just clarified on the index/newindex.

@nkoumchatzky
Copy link
Contributor

Got it.

@clementfarabet
Copy link
Collaborator

@alexbw got it, I removed indexCopy. Ok so let me try a few of these things then, last time I tried to do a copy in a narrowed tensor it failed.

@clementfarabet
Copy link
Collaborator

No, confirming that copy doesn't work as is. Pushing a new test to capture it: 0db08d8

@clementfarabet
Copy link
Collaborator

So adding back copy and indexCopy. These would work if you copied a tensor into them for which you didn't need grads... but in if you do, because you copy something that depends on a variable for which you want grads, then it needs to be defined.

Also I started a branch copy a few days ago to implement the copy operator, but I couldn't get it to work. What am i missing there?

@clementfarabet
Copy link
Collaborator

And once copy works, then __newindex__ is just syntactic sugar for :narrow():copy() and :select():copy().

@alexbw
Copy link
Collaborator Author

alexbw commented Oct 24, 2015

So you mean copy needs to be overridden to carry grads with the tensor?

@clementfarabet
Copy link
Collaborator

Yes it does! I pushed the code I have in master, which defines the 2 grads for torch.copy; it's failing with a A node type was not returned. This is either because a gradient was not defined, or the input is independent of the output.

@clementfarabet
Copy link
Collaborator

indexCopy will also need that, but you're right that indexFill won't. indexFill is just like any other constant source.

@alexbw
Copy link
Collaborator Author

alexbw commented Jan 8, 2016

Not really a very focused issue, closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants