#5548 closed defect (fixed)
fix that _hnf_mod segfaults sage completely
Reported by:  was  Owned by:  was 

Priority:  major  Milestone:  sage8.8 
Component:  linear algebra  Keywords:  
Cc:  burcin, jdemeyer  Merged in:  
Authors:  Alex Sun  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  8c43ba0 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description
teragon:algebras wstein$ sage   Sage Version 3.4, Release Date: 20090311   Type notebook() for the GUI, and license() for information.   sage: a = random_matrix(ZZ,16,4) sage: a._hnf_mod(100)  Unhandled SIGFPE: An unhandled floating point exception occured in SAGE. This probably occured because a *compiled* component of SAGE has a bug in it (typically accessing invalid memory) or is not properly wrapped with _sig_on, _sig_off. You might want to run SAGE under gdb with 'sage gdb' to debug this. SAGE will now terminate (sorry). 
Change History (19)
comment:1 Changed 13 years ago by
comment:2 Changed 13 years ago by
 Cc burcin added
comment:3 Changed 12 years ago by
 Cc jdemeyer added
 Report Upstream set to N/A
comment:4 Changed 9 years ago by
 Milestone changed from sage5.11 to sage5.12
comment:5 Changed 8 years ago by
 Milestone changed from sage6.1 to sage6.2
comment:6 Changed 8 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:7 Changed 8 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:8 Changed 3 years ago by
 Branch set to u/ghblackstones/5548
 Milestone changed from sage6.4 to sage8.8
comment:9 Changed 3 years ago by
 Commit set to 8c43ba044760d63e7c11188368343cc872458c6a
Branch pushed to git repo; I updated commit sha1. New commits:
8c43ba0  Check matrix is square

comment:10 Changed 3 years ago by
 Status changed from new to needs_review
Raised a ValueError? if the matrix is not square, and made a doctest out of the sample code in the description.
comment:11 Changed 3 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:12 Changed 3 years ago by
 Branch changed from u/ghblackstones/5548 to 8c43ba044760d63e7c11188368343cc872458c6a
 Resolution set to fixed
 Status changed from positive_review to closed
comment:13 followup: ↓ 14 Changed 3 years ago by
 Commit 8c43ba044760d63e7c11188368343cc872458c6a deleted
Shouldn't the added doctest block start with a TEST::
block header?
comment:14 in reply to: ↑ 13 Changed 3 years ago by
Replying to slelievre:
Shouldn't the added doctest block start with a
TEST::
block header?
I believe that a line ending in ::
signifies the code following it is a doctest, it doesn't necessarily need to be TEST::
. There are a few exceptions, like how MATH::
indicates latex, but there are many doctests that say something like "This fixes trac X::", followed by a doctest.
comment:15 Changed 3 years ago by
Although the code does get tested even without a TEST header, my understanding of the documentation strings section of Sage's coding basics is that testable examples in docstrings are usually placed inside "TEST" or "TESTS" or "EXAMPLE" or "EXAMPLES" blocks, which can go either
TESTS:: sage: 1 + 1 2 sage: 1 + 2 3
or
TESTS: We test that one plus one is two:: sage: 1 + 1 2 We test that one plus two is three:: sage: 1 + 2 3
(or similarly for TEST, EXAMPLE, EXAMPLES).
In addition, TEST and TESTS sections being meant more for developers than users, they get removed when building the documentation, unless one sets a dedicate environment variable:
SAGE_DOCBUILD_OPTS="includetestsblocks"
to keep them in.
Are there any doctests in docstrings elsewhere in the Sage sources that are not introduced by an EXAMPLE, EXAMPLES, TEST or TESTS header?
comment:16 Changed 3 years ago by
I agree with Samual that there should have been a TESTS:
block. I missed that during review. It's not a big problem though...
comment:17 Changed 3 years ago by
I see, I didn't realize that the TESTS::
header indicated a big block for all the tests, sorry for leaving that out. That being said, I did a quick search and found that there are some (very few) files that don't follow this...
 set_calculus_method() of sagemaster/src/sage/manifolds/manifold.py
 from_tamari_sorting_tuple() of sagemaster/src/sage/combinat/binary_tree.py ("EXEMPLES" instead of "EXAMPLES")
 init() of GenericGraphQuery? in sagemaster/src/sage/graphs/graph_database.py
 this file
My "quick search" was a bad Python script that has most likely left out other mistakes. Furthermore, some docstrings use "Examples" or "Tests" instead of the all uppercase version, and others have "Examples:" instead of "Examples::". I wasn't sure if they mattered so I didn't include them (also there were a lot of them).
This ticket is already closed, so let's not reopen it. Do you think it would be worth it to make a new ticket to search for and fix all these docstring mistakes?
comment:18 Changed 3 years ago by
A new ticket dedicated to fixing all missing or nonuppercase EXAMPLES and TESTS block headers would be nice.
comment:19 Changed 3 years ago by
If you want to fix those mistakes, great! Reviewing that should be trivial.
(the problem is that the input matrix isn't square...)