#5548 closed defect (fixed)
fix that _hnf_mod segfaults sage completely
Reported by: | was | Owned by: | was |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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: 2009-03-11 | | 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 sage-5.11 to sage-5.12
comment:5 Changed 8 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:6 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:7 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:8 Changed 3 years ago by
- Branch set to u/gh-black-stones/5548
- Milestone changed from sage-6.4 to sage-8.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/gh-black-stones/5548 to 8c43ba044760d63e7c11188368343cc872458c6a
- Resolution set to fixed
- Status changed from positive_review to closed
comment:13 follow-up: ↓ 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="--include-tests-blocks"
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 sage-master/src/sage/manifolds/manifold.py
- from_tamari_sorting_tuple() of sage-master/src/sage/combinat/binary_tree.py ("EXEMPLES" instead of "EXAMPLES")
- init() of GenericGraphQuery? in sage-master/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 non-uppercase 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...)