#15389: An algorithm for enumerating elements of bounded height in number fields
-------------------------------------+-------------------------------------
Reporter: dkrumm | Owner: dkrumm
Type: enhancement | Status: needs_work
Priority: minor | Milestone: sage-6.3
Component: number theory | Resolution:
Keywords: sage-days55 | Merged in:
Authors: David Krumm, John | Reviewers: Ben Hutz
Doyle | Work issues:
Report Upstream: N/A | Commit:
Branch: | bac2d779a1cba38ec787e1a9cbfe77e609ccd99a
u/jdoyle/bdd_height | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by bhutz):
* status: needs_review => needs_work
* reviewer: => Ben Hutz
Comment:
Mostly minors issues, but there are a few of them.
Some general comments first
- there are many instances of lines with trailing white space that need to
be removed.
- please merge in most recent beta as the branch on trac is weird and
seems to be deleting all of number_field.py, although that is not what the
downloaded branch does.
- put a REFERENCE block in one place, then reference in other places
- is the GRH parameter really needed as all it does is set the
number_field(True/False) switch? I would rather see that controlled by the
global switch and not the individual functions anyway. So, I'd remove the
parameter and mention the switch in the documentation.
- why is lll lower case and GRH upper case?
------------------------------
for the numberfield.py file:
- doc test error in number_field.py. Seems the elements are out of order.
- using keywords for the parameters would be better so they can be
specified independently.
- lll keyword is not used in the function
- output - an iterator **of number field elements***
------------------------------------
for the bdd_height.py file:
- It is standard to have some comments at the header to describe the
purpose of the file/GNU licence/authors/etc.
- itertools.product - is the only itertools you use, so only import that
one
- same with copy.copy
- same with real_mpfr.RealField
- the warning block is producing docbuild errors.
bdd_norm_pr_gens_iq:
- this is returning elements of K not ideals like the description says
- output - a dictionary **of principle ideals**, keyed by norm
bdd_height_iq:
- if bound is non-negative should always return 0, not empty list (current
skips 0 if the bound is < 1).
bdd_norm_pr_ideal_gens:
- output - a dictionary of ???
- returns elements not ideals as doc specifies
integer_points_in_polytope:
- check docs for [-interval_radius,interval_radius]^r
- # long time (40 s) -- is a very long doc test - does it need to be quite
this long or would a short long test do the same thing
- if you're allowing real numbers for the matrix shouldn't you have the
base_ring for the polyhedron be RR?
bdd_height:
- QQ to `QQ`, ``K`` to `K`. In general, things that need latex typesetting
(i.e., math) should be put in single quotes
- missed 0 for heights < 1
- same for these long tests - 290s is a *very* long time, please evaluate
whether such a long test is truly necessary.
- does the precision test by changing to QQ ever actually fail. I'm not
sure why this is a precision test since any real number to some finite
number of decimals places can be converted to a rational.
- 'principle ideals of bounded norm' are really generators of principle
ideals
--
Ticket URL: <http://trac.sagemath.org/ticket/15389#comment:12>
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.
