8000 Set `size` when `is_coalesced` is set in `torch.sparse_coo_tensor()` · Issue #145371 · pytorch/pytorch · GitHub
[go: up one dir, main page]
Skip to content

Set size when is_coalesced is set in torch.sparse_coo_tensor() #145371

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.

Sign up for GitHub

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

Jump to bottom
Closed
ILCSFNO opened this issue Jan 22, 2025 · 5 comments
Closed

Set size when is_coalesced is set in torch.sparse_coo_tensor() #145371

ILCSFNO opened this issue Jan 22, 2025 · 5 comments
Labels
module: sparse Related to torch.sparse triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@ILCSFNO
Copy link
Contributor
ILCSFNO commented Jan 22, 2025 •
edited by pytorch-bot bot
Loading

📚 The doc issue

The doc of torch.sparse_coo_tensor() shows its Parameters/Keyword Arguments as below:

size (list, tuple, or torch.Size, optional) – Size of the sparse tensor. If not provided the size will be inferred as the minimum size big enough to hold all non-zero elements.

is_coalesced (bool, optional) – WhenTrue, the caller is responsible for providing tensor indices that correspond to a coalesced tensor. If the check_invariants flag is False, no error will be raised if the prerequisites are not met and this will lead to silently incorrect results. To force coalescion please use coalesce() on the resulting Tensor. Default: None: except for trivial cases (e.g. nnz < 2) the resulting Tensor has is_coalesced set to False.

But when is_coalesced is set, whether it is None/True/False/..., size must be set properly, but document isn't noted or warned.

Repro

import torch
is_coalesced = True # choice: None, True, False
i = torch.tensor([[0, 1, 0], [1, 2, 3]])
v = torch.tensor([3.0, 4.0, 5.0])
s = (2, 3)
result = torch.sparse_coo_tensor(i, v, is_coalesced=is_coalesced) # always fail
# result = torch.sparse_coo_tensor(i, v, s, is_coalesced=is_coalesced) # always success

Outputs

TypeError: sparse_coo_tensor() received an invalid combination of arguments - got (Tensor, Tensor, is_coalesced=bool), but expected one of:
 * (object indices, object values, *, torch.dtype dtype = None, torch.device device = None, bool pin_memory = False, bool requires_grad = False, bool check_invariants = None)
 * (object indices, object values, tuple of ints size, *, torch.dtype dtype = None, torch.device device = None, bool pin_memory = False, bool requires_grad = False, bool check_invariants = None, bool is_coalesced = None)
 * (tuple of ints size, *, torch.dtype dtype = None, torch.device device = None, bool requires_grad = False, bool check_invariants = None)

Suggest a potential alternative/fix

So, a Note/Warning should be added to the doc of torch.sparse_coo_tensor() as shown below:

Note/Warning:
When is_coalesced is set, whether it is None/True/False/..., size must be set properly.

cc @alexsamardzic @nikitaved @pearu @cpuhrsch @amjames @bhosmer @jcaip @svekars @brycebortree @sekyondaMeta @albanD @mruberry @jbschlosser @walterddr @mikaylagawarecki
@cpuhrsch cpuhrsch added module: sparse Related to torch.sparse triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Jan 24, 2025
@pearu
Copy link
Collaborator
pearu commented Jan 24, 2025 •
edited
Loading

I suggest that the behavior of unspecified is_coalesced and specified is_coalesced=None to be identical. That is, as a fix, the signature

 (object indices, object values, *, 
   torch.dtype dtype = None, torch.device device = None,
   bool pin_memory = False, bool requires_grad = False,
   bool check_invariants = None)

ought to be extended with is_coalesced=None kw argument.

@ILCSFNO
Copy link
Contributor Author
ILCSFNO commented Jan 24, 2025

@pearu Oh sure, thanks. Yet I thought except this, may the binding relationship between size and is_coalesced also requires attention.

@pearu
Copy link
Collaborator
pearu commented Jan 24, 2025

@yinxinyang100 , size and is_coalesced are not related in a way that would prevent specifying any is_coalesced state that would be dependent on the specified/computed size, and vice-versa.

@ILCSFNO
Copy link
Contributor Author
ILCSFNO commented Jan 24, 2025

@pearu , I see, so by extending the first signature with is_coalesced=None, when size is unspecified but is_coalesced is specified like repro above, it can run well then. Thanks!

@ILCSFNO ILCSFNO mentioned this issue Feb 6, 2025
Open
@ILCSFNO ILCSFNO mentioned this issue May 2, 2025
Closed
@ILCSFNO
Copy link
Contributor Author
ILCSFNO commented May 2, 2025

@pearu PR opened here, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
module: sparse Related to torch.sparse triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix signature of torch.sparse_coo_tensor() ILCSFNO/pytorch
3 participants
@pearu @cpuhrsch @ILCSFNO
0