#13213: Comparisons in real number field
-------------------------------------------------+--------------------------
Reporter: vdelecroix | Owner: vdelecroix
Type: enhancement | Status: new
Priority: major | Milestone: sage-5.2
Component: number fields | Resolution:
Keywords: real number field, comparison | Work issues:
Report Upstream: N/A | Reviewers: Burcin Erocal
Authors: Vincent Delecroix | Merged in:
Dependencies: | Stopgaps:
-------------------------------------------------+--------------------------
Changes (by burcin):
* reviewer: => Burcin Erocal
Comment:
Thanks again for the patch. A few comments:
* reimplementing the comparison of quadratic number field elements is a
well defined unit of change itself. I suggest moving `floor()` and
`ceil()` implementations to a separate patch, even a different ticket.
* Do you really need to use rich comparisons? Working only with `__cmp__`
should simplify the case comparisons before the return statements and
avoid the new `python_object.pxi` include.
* I guess speed could be further improved by using an approximation (with
`double`s say) first and falling back to the exact computation if the
results are too close.
* `left.D` is of type `Integer`. Cython tries to do the right thing for
`<unsigned int> left.D`, but it would be better to use `mpz_mul()` with
`left.D.value`.
* `mpz_sgn()` returns 0 if the argument is `0`. You don't seem to cover
this case, though I couldn't find any example that returns wrong results.
* To avoid code duplication, it might be better change the if statement
to:
{{{
if mpz_sgn(i)*mpz_sgn(j) == 1:
# both i and j are positive or negative
mpz_mul(i, i, i)
mpz_mul(j, j, j)
mpz_mul_ui(j, j, <unsigned int> left.D)
test = mpz_cmp(i, j)
if mpz_sgn(i) == -1:
# both i and j are negative
test *= -1
}}}
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13213#comment:5>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
