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: sage-6.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:

Status badges

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 Travis Scrimshaw

I'm pretty sure the problem is this line in the sparse _dot_product_coerce_:

cdef dict e = (<FreeModuleElement_generic_sparse>right)._entries

which assumes the right vector is a sparse vector. I will fix this and post shortly.

comment:2 Changed 7 years ago by Travis Scrimshaw

Authors: Travis Scrimshaw
Branch: public/modules/dot_sparse_dense-19377
Commit: 2f7c7277fbc39d44c6a0d4dd56f789c3ed9c78d4
Status: newneeds_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 git

Commit: 2f7c7277fbc39d44c6a0d4dd56f789c3ed9c78d4d2f690be75c4f68df5ad10837edc1d1bdf7a4073

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

d2f690bFix for sparse dot dense vector.

comment:4 Changed 7 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

I think this would be much better:

try:
    e = (<FreeModuleElement_generic_sparse?>right)._entries
except TypeError:
    ...

comment:5 Changed 7 years ago by git

Commit: d2f690be75c4f68df5ad10837edc1d1bdf7a4073e92b704774396ffb2fa1a25e9ceb45e8a2c8f9f0

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

e92b704Following Jeroen's suggestion.

comment:6 Changed 7 years ago by Travis Scrimshaw

Status: needs_workneeds_review

Done.

comment:7 Changed 7 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

Not except: but except TypeError:

comment:8 Changed 7 years ago by git

Commit: e92b704774396ffb2fa1a25e9ceb45e8a2c8f9f026a0b6bc624b8eee81d35dfcf0dfa85b626a6245

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

26a0b6bTypeError...

comment:9 Changed 7 years ago by Travis Scrimshaw

Status: needs_workneeds_review

*facepalm* Thanks.

comment:10 Changed 7 years ago by John Palmieri

What does the question mark mean in

e = (<FreeModuleElement_generic_sparse?>right)._entries

comment:11 Changed 7 years ago by Travis Scrimshaw

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 John Palmieri

Reviewers: John Palmieri
Status: needs_reviewpositive_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 Jeroen Demeyer

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 Jeroen Demeyer

Reviewers: John PalmieriJohn 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 Volker Braun

Branch: public/modules/dot_sparse_dense-1937726a0b6bc624b8eee81d35dfcf0dfa85b626a6245
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.