Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to add an element to the diagonal #100

Closed
ldv1 opened this issue Dec 20, 2020 · 14 comments
Closed

How to add an element to the diagonal #100

ldv1 opened this issue Dec 20, 2020 · 14 comments
Labels

Comments

@ldv1
Copy link

ldv1 commented Dec 20, 2020

Hi,

I want to add a constant to the diagonal of a sparse matrix. Looking at GCNConv, I assume this is what SparseTensor.fill_diag does (it would be great if methods were documented).
With

import torch
from torch_sparse import SparseTensor
 
col = torch.tensor([1,1,5,0]).type(torch.long)
row = torch.tensor([2,5,3,0]).type(torch.long)
adj = SparseTensor(col=col, row=row)
A = adj.to_dense()
print(A)

I got as expected

tensor([[1., 0., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 0.],
        [0., 1., 0., 0., 0., 0.],
        [0., 0., 0., 0., 0., 1.],
        [0., 0., 0., 0., 0., 0.],
        [0., 1., 0., 0., 0., 0.]])

Further

adj = SparseTensor.fill_diag(adj, 4.)
print(adj.to_dense())

returns

tensor([[1., 0., 0., 0., 0., 0.],
        [0., 1., 0., 0., 0., 0.],
        [0., 1., 1., 0., 0., 0.],
        [0., 0., 0., 1., 0., 1.],
        [0., 0., 0., 0., 1., 0.],
        [0., 1., 0., 0., 0., 1.]])

So, the diagonal is filled with 1s, instead of 4s. What is wrong with my code?
Thanks for any help.

@rusty1s
Copy link
Owner

rusty1s commented Dec 20, 2020

It will only fill the diagonal with values in case value is present in adj:

col = torch.tensor([1,1,5,0]).type(torch.long)
row = torch.tensor([2,5,3,0]).type(torch.long)
value = torch.tensor([1, 1, 1, 1].type(torch.float)
adj = SparseTensor(col=col, row=row, value=value)

@ldv1
Copy link
Author

ldv1 commented Dec 20, 2020

Thanks for the prompt answer.
My code will then work with:


if not adj_t.has_value():
    adj = adj.fill_value(0.)
adj = SparseTensor.fill_diag(adj, 1234.)
print( adj.to_dense() )

So, I understand that value is sort of a placeholder. We first define the placeholder and then fill it.
May I ask why you opted for this strange construct, i.e. why fill_diag(adj,my_float), as the name implies, would not simply fill the diagonal elements with my_float?

What would be the simplest way to add to the diagonal instead of filling?

@rusty1s
Copy link
Owner

rusty1s commented Dec 20, 2020

I'm sorry for the lack of documentation :)

In general, SparseTensor supports two formats: One where we only care about connections (value=None), and one which is similar to any other sparse matrix library (value!=None). In case there exists no values, we simply want to add diagonal connections to the matrix.

There's no easy way yet to add diagonal entries to existing ones, since sum is currently not implemented.

@ldv1
Copy link
Author

ldv1 commented Dec 20, 2020

There's no easy way yet to add diagonal entries to existing ones, since sum is currently not implemented

That's bad news. I wanted to extent SIGN to more general propagation matrices. For that I need to add to the diagonal of an adjacency matrix.

@rusty1s
Copy link
Owner

rusty1s commented Dec 20, 2020

But for that, adding diagonal entries is sufficient, isn't it? No need to sum them up in case there already exists some.

@ldv1
Copy link
Author

ldv1 commented Dec 20, 2020

The idea is to define a continuum of propagation matrices. To make the problem more easily understandable, I will push a PR.

@ldv1
Copy link
Author

ldv1 commented Jan 16, 2021

We have a set_diag method. Is there a get_diag?

@rusty1s
Copy link
Owner

rusty1s commented Jan 18, 2021

Getting diagonal entries is quite straightforward:

row, col, value = A.coo()
diag_values = value[row == col]

@ldv1
Copy link
Author

ldv1 commented Jan 18, 2021

Thanks.
Why not a new function set_diag in diag.py ? That might be useful for others as well.

@rusty1s
Copy link
Owner

rusty1s commented Jan 19, 2021

Just pushed mat.get_diag() to master, see here.

@ldv1
Copy link
Author

ldv1 commented Jan 19, 2021

Just what I needed. Thanks.

@ldv1
Copy link
Author

ldv1 commented Jan 21, 2021

Currently, we can only set the diagonal elements of a sparse matrix by specifying a scalar, right ? Would it be possible to specify a tensor instead ?

@rusty1s
Copy link
Owner

rusty1s commented Jan 22, 2021

You can either fill with a scaler via fill_diag or with a tensor via set_diag.

@github-actions
Copy link

This issue had no activity for 6 months. It will be closed in 2 weeks unless there is some new activity. Is this issue already resolved?

@github-actions github-actions bot added the stale label Sep 16, 2021
@github-actions github-actions bot closed this as completed Oct 1, 2021
RexYing pushed a commit to RexYing/pytorch_sparse that referenced this issue Apr 26, 2022
* read_csv

* update

* update

* update

* update

* update

* add test

* update

* re-add column tuple
yanbing-j pushed a commit to yanbing-j/pytorch_sparse that referenced this issue Aug 18, 2022
fix: fix errors regarding Reducer functionalities in segment.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants