#14239: symbolic radical expression for algebraic number
-------------------------------------+-------------------------------------
Reporter: gagern | Owner: davidloeffler
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-6.4
Component: number fields | Resolution:
Keywords: | Merged in:
Authors: Martin von Gagern | Reviewers: Marc Mezzarobba,
Report Upstream: N/A | Jeroen Demeyer
Branch: | Work issues:
u/jdemeyer/ticket/14239 | Commit:
Dependencies: #17495, #16964 | 24b00c640ccc096afa24da92e29cf2c1372628e2
| Stopgaps:
-------------------------------------+-------------------------------------
Comment (by gagern):
Thanks for the rationale from comment:35. Can you add test cases for each
of these benefits? I see you already wrote one for the case where the
generator has no radical expression. So far none of the existing test
cases covers the other two benefits, though, since none had to be changed.
Since you removed the caching of symbolic expressions in the `__symbolic`
member of the number field element, I think you should also remove said
member from the `number_field_element.pxd` since it is without use now.
I'm still not happy that now we'll have to fix two other issues before we
can evaluate what you did here and see whether the (quite valid) gains are
worth the performance penalty due to the removal of caching. Or, to be
more precise, I'm unhappy that my own modifications are blocked together
with this.
I also have some doubts about the `refine_embedding` approach. As far as I
can see, the refined embedding ''should'' get cached in the number field,
but I'd prefer to see that verified by a test case. The choice which of
these embeddings to use doesn't seem to be cached, so that matching would
be repeated for every conversion. Not sure how costly that is, but I would
prefer if we could cache the algebraic embedding which matches the
specified complex embedding explicitely. Looking at `embeddings` I also
think that `K['x'](self.defining_polynomial()).roots()` is probably bad,
since singular can't deal with polynomials with algebraic coefficients.
Shouldn't this be `self.defining_polynomial().roots(K)` instead? The
subsequent `r.sort()` seems superfluous, since roots are to my knowledge
always returned in sorted order.
--
Ticket URL: <http://trac.sagemath.org/ticket/14239#comment:64>
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.
