Opened 13 years ago

Closed 13 years ago

#2204 closed defect (fixed)

[with spkg, with positive review] Integrate Karim Belabas's HNF bug fix for pari, add 64 bit OSX support

Reported by: mabshoff Owned by: mabshoff
Priority: major Milestone: sage-2.10.2
Component: packages: standard Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Reported by William via the pari bug tracker:

---------- Forwarded message ----------
From: Karim Belabas <Karim.Belabas@math.u-bordeaux1.fr>
Date: Feb 18, 2008 2:49 AM
Subject: Re: Bug#741: Fwd: bug in PARI's mathnf function
To: William Stein <wstein@gmail.com>, 741-close@pari.math.u-bordeaux.fr


* William Stein [2008-02-16 23:47]:
> > Package: pari
> > Version: 2.3.3
[...]
> > PARI sometimes puts negative numbers in the *output* of mathnf(a, 1)[0],
> > which is a bug.

Indeed.

> >   * I didn't use mathnf(b) directly (the default option), since
> > already for a 20x18 matrix it
> > is too slow to be useful.

As documented.

[ Actually, I am going to change this: I don't see the point in defaulting
to a slow routine; let mathnf choose depending on the matrix size.
It should either call matdetint + mathnfmod, or mathnf(,1) ]

> >   * I'm guessing maybe mathnf(b, 1) uses the modular hnf modulo the
> >   determinant.

Actually, no; mathnfmod does that.

> > If not, maybe you just need to add mutliples of rows until everything
> > is correctly normalized.

We do that, with one optimization too many which sometimes cancelled the
normalization step ( when the kernel is non trivial and we are "lucky":
a coefficient which we want to set to 0 is already 0 ).

It is fixed in both stable and unstable branches. I am attaching the
(trivial) patch.

Thanks for your report !

    K.B.
--
Karim Belabas                  Tel: (+33) (0)5 40 00 26 17
IMB, Universite Bordeaux 1     Fax: (+33) (0)5 40 00 69 50
351, cours de la Liberation    http://www.math.u-bordeaux.fr/~belabas/
F-33405 Talence (France)       http://pari.math.u-bordeaux.fr/  [PARI/GP]

Attachments (2)

hnf.patch (644 bytes) - added by mabshoff 13 years ago.
This patch needs to be applied to the pari.spkg
trac-2204.patch (1.8 KB) - added by was 13 years ago.
apply this patch to the sage hg repo; it adds a test to matrix/tests.py to ensure that the bug is (and stays) fixed.

Download all attachments as: .zip

Change History (6)

Changed 13 years ago by mabshoff

This patch needs to be applied to the pari.spkg

comment:1 Changed 13 years ago by mabshoff

  • Status changed from new to assigned
  • Summary changed from Integrate Karim Belabas's HNF bug fix for pari to [with spkg, needs review] Integrate Karim Belabas's HNF bug fix for pari

The updated spkg at

http://sage.math.washington.edu/home/mabshoff/release-cycles-2.10.2/alpha1/pari-2.3.3.p0.spkg

contains the patch, adds support for 64 bit OSX. Builds fine on Linux and OSX.

Cheers,

Michael

comment:2 Changed 13 years ago by mabshoff

  • Summary changed from [with spkg, needs review] Integrate Karim Belabas's HNF bug fix for pari to [with spkg, needs review] Integrate Karim Belabas's HNF bug fix for pari, add 64 bit OSX support

Changed 13 years ago by was

apply this patch to the sage hg repo; it adds a test to matrix/tests.py to ensure that the bug is (and stays) fixed.

comment:3 Changed 13 years ago by mabshoff

  • Summary changed from [with spkg, needs review] Integrate Karim Belabas's HNF bug fix for pari, add 64 bit OSX support to [with spkg, with positive review] Integrate Karim Belabas's HNF bug fix for pari, add 64 bit OSX support

trac-2204.patch looks good to me. With it applied and the new pari.spkg doctests in sage/matrix/tests.py pass, so positive review (also from was for the spkg).

Cheers,

Michael

comment:4 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from assigned to closed

Merged in Sage 2.10.2.alpha1

Note: See TracTickets for help on using tickets.