#15976: IntegerLattice class
-------------------------------------+-------------------------------------
Reporter: malb | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.3
Component: linear algebra | Resolution:
Keywords: | Merged in:
Authors: Martin Albrecht | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
public/ticket/15976 | a561d3691fe0cdfb824c6313fb254c7220c430b6
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by dkrenn):
I've uploaded a couple of minor changes made during review (most of them
are whitespace corrections, PEP8, rewriting of long lines, adding ``...``
in documentation.
Here some further comments:
*
{{{
diff --git a/src/doc/common/conf.py b/src/doc/common/conf.py
index 25dd9f0..ff9efe3 100644
--- a/src/doc/common/conf.py
+++ b/src/doc/common/conf.py
@@ -296,6 +296,13 @@ latex_elements['preamble'] = r"""
\DeclareUnicodeCharacter{2510}{+}
\DeclareUnicodeCharacter{2514}{+}
\DeclareUnicodeCharacter{2518}{+}
+\DeclareUnicodeCharacter{03BC}{\mu}
+\DeclareUnicodeCharacter{03B4}{\delta}
+\DeclareUnicodeCharacter{03B7}{\eta}
+\DeclareUnicodeCharacter{03BB}{\lambda}
+\DeclareUnicodeCharacter{2266}{\le}
+\DeclareUnicodeCharacter{221A}{\sqrt}
}}}
I'd appreciate if someone else can make a short review on this part, since
I do not know what should be done. (It seems that the code works, but I
just don't know if this is how it is done; probably yes).
* changes in fplll.*: The changes look fine for me and the code seem to
work. I have to say, I am not really familiar with the fplll interface, so
maybe someone else can look at this code (but for me it is positive_review
on this part).
*
{{{
diff --git a/src/sage/libs/fplll/fplll.pyx b/src/sage/libs/fplll/fplll.pyx
index 59cd53e..b28f5c9 100644
--- a/src/sage/libs/fplll/fplll.pyx
+++ b/src/sage/libs/fplll/fplll.pyx
@@ -18,6 +18,7 @@ AUTHORS:
+# Copyright (C) 2014 Martin Albrecht
<[email protected]>
^^^^ ^^^^
}}}
Is this on purpose?
* fplll.pyx: OUTPUT-blocks are missing (although, most of them will state
"Nothing") (The developerguide does not say that those can be skipped if
nothing is returned...)
* fplll.pyx: Sometimes there are empty lines between two input-
descriptions (in INPUT-block), sometimes not. This should be done in the
same way everywhere.
* fplll.pyx: def BKZ: In the INPUT-block you can find the following:
{{{
- ``block_size`` - 0 < block size <= nrows
- ``no_lll`` -
- ``bounded_lll`` -
}}}
The first line looks a bit weird, since this is a mixture of math and
verbated text written plain. Maybe add ``...`` or `...` there?
The last two don't have any description.
* I am not sure which of the following one should prefer for {{{default:
0}}}: `0` or ``0`` or 0?
(I have a slight preference for `0`). But this should be done in the same
way everywhere.
* matrix_integer_dense.pyx:
{{{
- ``prune`` - The optional parameter ``prune`` can be set to any
positive number to invoke the Volume Heuristic from [Schnorr
and
Horner, Eurocrypt '95]. This can significantly reduce the
running
time, and hence allow much bigger block size, but the quality
of the
reduction is of course not as good in general. Higher values of
``prune`` mean better quality, and slower running time. When
``prune`` == 0, pruning is disabled. Recommended usage: for
``block_size`` = 30, set 10 = ``prune`` = 15.
}}}
The last few lines (== 0, = 30, 10 = ``prune`` = 15) look a bit weird.
* matrix_integer_dense.pyx:
{{{
fpLLL SPECIFIC INPUTS:
- ``precision`` - bit precision to use if ``fp=='rr'`` (default: 0
for automatic choice)
= here ^^^^^^^^ ?
}}}
* in matrix_integer_dense.pyx: Sometimes there is {{{OUTPUT: an
integer}}}. Shouldn't this be in separate lines?
* A couple of times the descriptions in the INPUT-block end with a full-
stop; most of the times not.
* free_module_integer.py: Some missing OUTPUT-blocks
*
{{{
SCORE src/sage/libs/fplll/fplll.pyx: 89.5% (17 of 19)
Missing documentation:
* line 492: def HKZ(self)
Missing doctests:
* line 499: def shortest_vector(self, method=None)
}}}
Apart from those minor things, everything else looks good.
--
Ticket URL: <http://trac.sagemath.org/ticket/15976#comment:38>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
