Opened 7 years ago
Closed 7 years ago
#19377 closed defect (fixed)
dot product of sparse vector with dense vector: segfault
Reported by:  John Palmieri  Owned by:  

Priority:  critical  Milestone:  sage6.9 
Component:  linear algebra  Keywords:  
Cc:  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  John Palmieri, Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  26a0b6b (Commits, GitHub, GitLab)  Commit:  26a0b6bc624b8eee81d35dfcf0dfa85b626a6245 
Dependencies:  Stopgaps: 
Description
sage: v = random_vector(QQ, 5, sparse=True) sage: w = random_vector(QQ, 5, sparse=False) sage: v.dot_product(w)  Unhandled SIGSEGV: A segmentation fault occurred in Sage. This probably occurred because a *compiled* component of Sage has a bug in it and is not properly wrapped with sig_on(), sig_off(). Sage will now terminate. 
Change History (15)
comment:1 Changed 7 years ago by
comment:2 Changed 7 years ago by
Authors:  → Travis Scrimshaw 

Branch:  → public/modules/dot_sparse_dense19377 
Commit:  → 2f7c7277fbc39d44c6a0d4dd56f789c3ed9c78d4 
Status:  new → needs_review 
Easy enough fix (even if it many not be the "best", but I would argue that if this is a bottleneck, then one needs to be more careful about dense/sparse vectors).
comment:3 Changed 7 years ago by
Commit:  2f7c7277fbc39d44c6a0d4dd56f789c3ed9c78d4 → d2f690be75c4f68df5ad10837edc1d1bdf7a4073 

Branch pushed to git repo; I updated commit sha1. New commits:
d2f690b  Fix for sparse dot dense vector.

comment:4 Changed 7 years ago by
Status:  needs_review → needs_work 

I think this would be much better:
try: e = (<FreeModuleElement_generic_sparse?>right)._entries except TypeError: ...
comment:5 Changed 7 years ago by
Commit:  d2f690be75c4f68df5ad10837edc1d1bdf7a4073 → e92b704774396ffb2fa1a25e9ceb45e8a2c8f9f0 

Branch pushed to git repo; I updated commit sha1. New commits:
e92b704  Following Jeroen's suggestion.

comment:7 Changed 7 years ago by
Status:  needs_review → needs_work 

Not except:
but except TypeError:
comment:8 Changed 7 years ago by
Commit:  e92b704774396ffb2fa1a25e9ceb45e8a2c8f9f0 → 26a0b6bc624b8eee81d35dfcf0dfa85b626a6245 

Branch pushed to git repo; I updated commit sha1. New commits:
26a0b6b  TypeError...

comment:10 Changed 7 years ago by
What does the question mark mean in
e = (<FreeModuleElement_generic_sparse?>right)._entries
comment:11 Changed 7 years ago by
For checking the type. See the type checking section in http://docs.cython.org/src/reference/language_basics.html.
comment:12 Changed 7 years ago by
Reviewers:  → John Palmieri 

Status:  needs_review → positive_review 
This fixes the problem for me. I don't know if it's the best way to do it, but I agree with comment #2, so I'll give it a positive review.
comment:13 Changed 7 years ago by
John: essentially
cdef t a = <t?>b
is equivalent to
if not isinstance(b, t): raise TypeError cdef t a = <t>b
comment:14 Changed 7 years ago by
Reviewers:  John Palmieri → John Palmieri, Jeroen Demeyer 

I didn't run doctests (I hope any of you did) but the change does indeed look good.
comment:15 Changed 7 years ago by
Branch:  public/modules/dot_sparse_dense19377 → 26a0b6bc624b8eee81d35dfcf0dfa85b626a6245 

Resolution:  → fixed 
Status:  positive_review → closed 
I'm pretty sure the problem is this line in the sparse
_dot_product_coerce_
:which assumes the right vector is a sparse vector. I will fix this and post shortly.