Opened 7 years ago
Closed 6 years ago
#13531 closed defect (fixed)
short_vector_list_up_to_length is slow and wrong
Reported by: | mraum | Owned by: | justin |
---|---|---|---|
Priority: | major | Milestone: | sage-5.7 |
Component: | quadratic forms | Keywords: | |
Cc: | Merged in: | sage-5.7.beta3 | |
Authors: | Martin Raum | Reviewers: | Volker Braun |
Report Upstream: | Workaround found; Bug reported upstream. | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
- The method short_vector_list_up_to_length does not return a list of vectors as claimed in the documentation.
- Even with lattices of rank 6 and determinant 3 it takes really long to get the result. This is not PARI's fault (which is called in the background), but pexpect's.
- In some cases no result will be returned because to many GP queries are performed. This exceeds the maximal number of GP's sage[...] variables.
All this is fixed (the speed only in parts) by the attached patch.
This is PARI Bug#1394, fixed upstream in commit 2d4455dc481cc0f0b09900e97c4fbb2e87b8db4d.
Apply trac_13531-short_vectors-v2.patch
Attachments (1)
Change History (10)
comment:1 Changed 7 years ago by
- Status changed from new to needs_review
comment:2 Changed 6 years ago by
comment:3 Changed 6 years ago by
Can you add a doctests for the Pari bug and a link to the upstream bug report? Presumably we'll want to remove your workaround when it is fixed.
Changed 6 years ago by
comment:4 Changed 6 years ago by
The test for the too long vectors is marked long time, because on my system it takes about 140 seconds. Currently, this is the best example I have.
I have looked for the bug report, and I can't find it. I might have forgotten to send it, so I have just sent it. I post the bug number as soon as possible.
comment:5 Changed 6 years ago by
- Description modified (diff)
- Report Upstream changed from N/A to Workaround found; Bug reported upstream.
comment:6 Changed 6 years ago by
- Description modified (diff)
I got this reply:
it seems this bug has disappeared in the development branch in rev 2d4455dc:
commit 2d4455dc481cc0f0b09900e97c4fbb2e87b8db4d
Author: Karim Belabas <Karim.Belabas@…>
Date: Sun Jul 17 11:27:48 2011 +0000
minor cleanup minim0()
comment:7 Changed 6 years ago by
- Reviewers set to Volker Braun
- Status changed from needs_review to positive_review
Thanks!
comment:8 Changed 6 years ago by
- Description modified (diff)
comment:9 Changed 6 years ago by
- Merged in set to sage-5.7.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
I had to make even further changes, since Pari sometimes returns incorrect results. I will send a report to upstream, and I have built in a work around for the moment. For the record, I reproduce the corresponding example here.
gives the following: