#19152: {Real,Complex}Ball: Miscellaneous fixes and improvements
-------------------------------------+-------------------------------------
Reporter: mmezzarobba | Owner:
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.10
Component: numerical | Resolution:
Keywords: | Merged in:
Authors: Marc Mezzarobba | Reviewers: Clemens Heuberger
Report Upstream: N/A | Work issues:
Branch: u/cheuberg/19152 | Commit:
-arb-misc | 16a04a07c63349d3792ed8c538a7d60e5adfde5e
Dependencies: #19063 | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by mmezzarobba):
Hello Jeroen,
Thank you very much for your remarks.
Replying to [comment:15 jdemeyer]:
> 2. You can remove the `include_dirs=[SAGE_INC + '/flint'])` from
`src/module_list.py`.
Are you sure? I get:
{{{
[2/2] gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -fPIC
-I/home/marc/co/sage/local/include
-I/home/marc/co/sage/local/include/python2.7
-I/home/marc/co/sage/local/lib/python2.7/site-packages/numpy/core/include
-I/home/marc/co/sage/src -I/home/marc/co/sage/src/sage/ext
-I/home/marc/co/sage/src/build/cythonized
-I/home/marc/co/sage/src/build/cythonized/sage/ext
-I/home/marc/co/sage/local/include/python2.7 -c
/home/marc/co/sage/src/build/cythonized/sage/rings/real_arb.c -o
build/temp.linux-
x86_64-2.7/home/marc/co/sage/src/build/cythonized/sage/rings/real_arb.o
-fno-strict-aliasing -w
In file included from
/home/marc/co/sage/src/build/cythonized/sage/rings/complex_ball_acb.c:310:0:
/home/marc/co/sage/local/include/mag.h:36:19: fatal error: flint.h: No
such file or directory
compilation terminated.
In file included from
/home/marc/co/sage/src/build/cythonized/sage/rings/real_arb.c:309:0:
/home/marc/co/sage/local/include/mag.h:36:19: fatal error: flint.h: No
such file or directory
}}}
> 3. Ideally, the type declarations in the arb `.pxd` files should be in a
different file: try to follow the model of `src/sage/libs/gmp` for
example. Then you should add `# distutils: libraries = arb` to the files
with library functions (i.e. not types).
A difference is that arb comes with several `.h` files (one for each
module, with a more or less well-defined dependency graph): are you
suggesting to put all the type definitions in a single `.pxd` file
nevertheless?
> 4. The `__hash__` for `RBF` is bad:
> {{{
> sage: from sage.rings.real_arb import RBF
> sage: hash(RBF(0.6))
> 1048577
> sage: hash(RBF(0.7))
> 1048577
> sage: hash(RBF(0.8))
> 1048577
> sage: hash(RBF(0.9))
> 1048577
> }}}
Fixed (see above).
> 5. For `CBF.__hash__`, what's the point of `>> 7`?
The idea was just to avoid that the λ(1+i) all hash to zero...
> 6. This is a very strange way to handle exceptions, what's wrong with
`raise`?
> {{{
> if arf_get_mpfr(rad.value, tmp, GMP_RNDN):
> abort()
> }}}
I don't remember for sure. I guess I may have wanted to make sure that
errors raised by `arf_get_mpfr()` itself and errors raised manually give
the same exception, and to avoid repeating the error message.
> 7. Replace the `GMP_RND` macros by `MPFR_RND`.
Done.
> 9. What's with this comment?
> {{{
> # FIXME: RBF is not even associative, but
CompletionFunctor
> # only works with rings, and other coercion features
require
> # methods like is_integral_domain() to be defined
> category=category or
sage.categories.fields.Fields().Infinite())
> }}}
> Your class models a mathematical field (real or complex numbers), so I
don't see the problem. How would you want to fix this?
Well, people seem to disagree about the meaning of categories in Sage, and
some people apparently assume that a parent that belongs to the category
of fields will satisfy the axioms of fields, not just vaguely model a
field. This is particularly harmful in the context of interval arithmetic.
In the long term, I'd want to fix this by (i) clarifying the “global
conventions” that need to be used consistently across the whole sage
codebase and (ii) depending on the outcome, perhaps creating categories
for “inexact fields” and the like.
> 10. Now that all this is standard, can you add `RBF`, `RealBallField`,
`CBF` and `ComplexBallField` to the global namespace and then remove all
doctest lines of the form `from sage.rings.real_arb import RBF`.
I'm no fan of adding anything to the global namespace, but I'll do it if
you insist.
> 11. You should really avoid this: `include "sage/ext/python.pxi"`, just
`cimport` what you need.
Done.
> 12. What's the difference between `contains_exact` and `__contains__`? I
don't understand why you need both these methods.
The difference is that `contains_exact()` works for fewer kinds of
objects, but is guaranteed not to return false negatives, while
`__contains__()` just does the check at the parent's precision. Of course,
one could do with `__contains__()` alone, but then it would be too easy to
call `x in b` by accident with an `x` for which it may not return the
correct result.
> 13. When raising exceptions, add a string: don't just `raise TypeError`,
but `raise TypeError("useful message here")`
Done. (But I didn't include a message because I couldn't think of a useful
message, so I doubt the new version is much better...)
> 14. You write `.. SEEALSO:: :meth:`abs` but there is no such method.
Indeed, I forgot to remove these references, thanks!
> 15. I still don't understand why `below_abs` and `above_abs` return a
ball instead of a real number. At the very least, this should be
documentated in an `OUTPUT:` block in those methods.
Why: because I often compute upper or lower bounds on quantities
represented by balls and then do arithmetic operations on the bounds that
may result in rounding errors. Representing the bounds themselves as thin
intervals is pretty convenient then. I also find that choice more
consistent with the rest of the interface of real/complex balls...
I added OUPUT blocks, please tell me if there is anything else you feel
these blocks should say.
> 16. This is confusing documentation: `Return the right endpoint of this
ball, viewed as an interval.` I would read it like this method returns an
interval. Why not `Return the right endpoint of this ball.`?
Indeed! See my answer to Clemens above.
> 17. I don't understand the purpose of this comment: `# TODO: once CBF is
there, also wrap functions that only exist in acb*`
There are special functions in arb that are only implemented for complex
arguments, but that would make sense on sage real balls.
> 18. I don't understand what this means: `Return a copy of this ball
rounded to the precision of the parent.`
An arb ball has no intrinsic precision, and (more or less because of
that), an element of `RealBallField(p)` can have a midpoint that doesn't
fit on `p` bits. The `round()` method rounds such elements to the
precision of their parent.
--
Ticket URL: <http://trac.sagemath.org/ticket/19152#comment:18>
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.
