#14239: symbolic radical expression for algebraic number
-------------------------------------+-------------------------------------
Reporter: gagern | Owner: davidloeffler
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.5
Component: number fields | Resolution:
Keywords: | Merged in:
Authors: Martin von | Reviewers: Marc Mezzarobba,
Gagern, Jeroen Demeyer | Jeroen Demeyer
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/gagern/ticket/14239 | 4626286463c734f931ecc7405285dc7bc1b58772
Dependencies: #17495, #16964 | Stopgaps:
-------------------------------------+-------------------------------------
Changes (by {'newvalue': u'Martin von Gagern, Jeroen Demeyer', 'oldvalue':
u'Martin von Gagern'}):
* author: Martin von Gagern => Martin von Gagern, Jeroen Demeyer
Comment:
Replying to [comment:99 vdelecroix]:
> What is the point of
> {{{
> - Test :trac:`14895`::
> -
> - sage: K.<sqrt2> = QuadraticField(2)
> + sage: K.<sqrt2> = QuadraticField(2) # :trac:`14895`
> }}}
That one is from
[http://git.sagemath.org/sage.git/commit/?h=4626286463c734f931ecc7405285dc7bc1b58772
4626286] by Jeroen, but you have the patch reversed. I assume that
{{{:trac:`…`}}} work in plain text but not in doctest comments, so this
made a lot of sense to me.
> I really do not like the attribute symbolic. Why this result needs to be
cached? You can possibly cache '''in the parent''' a symbolic version of
the generator.
Could it be you are seeing the wrong patch direction here as well? We are
''dropping'' the `__symbolic` member and its caching functionality, since
we don't expect to need it except in rare cases as a temporary solution.
> The fact that `SR(a)` when `a` is a number field element returns
possibly a numerical approximation is a very bad regression. You switch
from something exact to something approximative.
Yes, you are definitely seeing the patch in the wrong direction. A good
thing that you pasted the patch lines above, otherwise I would have ben
thoroughly confused.
----
Replying to [comment:100 vdelecroix]:
> What is the point of looking for an approximation
>
> {{{
> k = ( K._n()*CC(K.gen()).log() / CC(two_pi_i) ).real().round() # n ln z
/ (2 pi i)
> }}}
I must confess that I was a bit unhappy with that myself when I read it.
But the code just changed place and location, it wasn't really changed.
And I figure that if you ever have a cyclotomic field of such a great
order that you are affected by rounding errors here, its dimension would
be high enough that you'd not get any real work done with it in any case.
So I decided not to complain or modify this, since it's not directly
related to the issue at hand and I feard that discussing one more item
might prevent stuff from getting accepted.
Are you sure you want this handled with this ticket here? If so I'll have
to work out how to find the right `k` using exact arithmetic. I guess the
integers are all there somewhere, but I'll have to look closer. It's been
a while.
----
Replying to [comment:101 vdelecroix]:
> Instead of `embedding.im_gens()[0]` you can use the clearer
`embedding.gen_image()`.
Thanks, that should be a quick and obvious improvement. Will push a commit
to that effect once I know whether to address the approximation issue
above as well.
--
Ticket URL: <http://trac.sagemath.org/ticket/14239#comment:102>
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.
