Skip to content

Conversation

yueyinqiu
Copy link
Contributor

close #1291

@yueyinqiu
Copy link
Contributor Author

yueyinqiu commented Apr 26, 2024

By the way how about the circular reference?

using TorchSharp;

for (; ; )
{
    var t = torch.zeros(1000);
    t.grad = t;
    // t.set_grad(t);
}

PyTorch does not allow that:

image

However if the trick is played on two tensors, it's ok:

import torch

t = torch.zeros(1000)
g = torch.zeros(1000)
t.grad = g
g.grad = t

I have no idea how to deal with that... Just keep it leaking?

@yueyinqiu yueyinqiu marked this pull request as ready for review April 26, 2024 06:24
return new Tensor(res);
}
set {
NativeMethods.THSTensor_set_grad(Handle, value?.Handle ?? IntPtr.Zero);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the detach from dispose scope?
You also removed the MoveToOtherDisposeScope which was listed above in the to function.

Copy link
Contributor Author

@yueyinqiu yueyinqiu Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that I have misunderstood something... I have reverted them.

Copy link
Contributor Author

@yueyinqiu yueyinqiu Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaltielshmid

I have reviewed it again and I'm now more confident about the removal. Could you please check my comment here and the new commit about this?

@yueyinqiu yueyinqiu marked this pull request as draft April 26, 2024 14:36
@yueyinqiu yueyinqiu marked this pull request as ready for review April 26, 2024 14:48
@yueyinqiu yueyinqiu marked this pull request as draft April 26, 2024 14:50
@yueyinqiu
Copy link
Contributor Author

Perhaps I shall create a new pull request for this...

@yueyinqiu yueyinqiu closed this Apr 26, 2024
@yueyinqiu
Copy link
Contributor Author

#1299

@yueyinqiu yueyinqiu mentioned this pull request Apr 27, 2024
@yueyinqiu yueyinqiu deleted the grad branch May 1, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

should Tensor.grad be a property instead of a method?

2 participants