#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 | 142e8da1f1b387a5a2ea2d278f29e91d97930aac
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by malb):
Replying to [comment:38 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.
Thanks!
> 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).
This essentially allows to write UTF-8 characters like δ directly, which
makes the documentation easier to read.
>
> *
> {{{
> 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?
Fixed.
> * 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...)
I added those.
> * 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.
I should have fixed all of those.
> * 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.
Fixed.
> * 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.
I opted for ``0`` so that ``True`` etc. look the same?
> * 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.
Fixed.
> * matrix_integer_dense.pyx:
> {{{
> fpLLL SPECIFIC INPUTS:
>
> - ``precision`` - bit precision to use if ``fp=='rr'`` (default:
0 for automatic choice)
> = here ^^^^^^^^ ?
> }}}
I don't understand this point, fp='rr' is a valid parameter.
> * in matrix_integer_dense.pyx: Sometimes there is {{{OUTPUT: an
integer}}}. Shouldn't this be in separate lines?
I fixed those.
> * A couple of times the descriptions in the INPUT-block end with a full-
stop; most of the times not.
I tried to unify those.
> * free_module_integer.py: Some missing OUTPUT-blocks
Fixed.
> *
> {{{
> 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)
> }}}
Fixed.
> Apart from those minor things, everything else looks good.
--
Ticket URL: <http://trac.sagemath.org/ticket/15976#comment:40>
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.
