Opened 3 years ago

Closed 9 months ago

#24302 closed enhancement (fixed)

Different of an Element of a Number Field

Reported by: cgeiger Owned by:
Priority: minor Milestone: sage-9.2
Component: number fields Keywords: different, element
Cc: ​tscholl2, klui Merged in:
Authors: Caleb Geiger, Travis Scholl Reviewers: John Cremona, Vincent Delecroix, Samuel Lelièvre
Report Upstream: N/A Work issues:
Branch: cb841de (Commits, GitHub, GitLab) Commit: cb841def3d47c9b35cdbe875f28067476ad001ac
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

We implement the different of a number field element.

Change History (14)

comment:1 Changed 3 years ago by cgeiger

  • Branch set to u/cgeiger/different_of_an_element_of_a_number_field

comment:2 Changed 3 years ago by cgeiger

  • Authors set to caleb geiger
  • Cc ​tscholl2 klui added
  • Commit set to 739ea8dd1635b881e91a40d7e8de59ff5cb1fdeb
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

739ea8dImplemented the different of a number field element.

comment:3 Changed 3 years ago by vdelecroix

  • Milestone changed from sage-8.1 to sage-8.2
  • Status changed from needs_review to needs_work

The different ideal is something standard (and useful to study ramifications in the ring of integers). What is the different of an element useful for? What would be the usage of these functions?

Concerning the code:

  1. It is a very bad idea to introduce a method that is not working (here different). The reason is that such method appear when you do tab-completion and provokes more confusion than anything else. Just don't implement it.
  1. It would be much simpler to have only one method called different which accept a base_field argument so that
    • a.different() would always be relative to the base field
    • a.different(base_field=QQ) would be relative to QQ I also don't see why you should not allow base_fied to be any subfield.
  1. As written in developer guide the documentation of a function should start with a one line description. An optional paragraph can follow.
  1. In the documentation, you should say precisely what is a and what is f.

comment:4 Changed 11 months ago by tscholl2

  • Authors changed from caleb geiger to caleb geiger, Travis Scholl
  • Branch changed from u/cgeiger/different_of_an_element_of_a_number_field to u/tscholl2/24302
  • Commit changed from 739ea8dd1635b881e91a40d7e8de59ff5cb1fdeb to 89957f6bbec3587ca1bf6e9ee657dd4b846e9fb2
  • Milestone changed from sage-8.2 to sage-9.1
  • Priority changed from major to minor
  • Status changed from needs_work to needs_review

The different of an element comes up for me every once in a while. Usually I want to see if the different of an element generates the different ideal. So having a .different() method makes that a tad easier.

I attempted to incorporate the comments into Caleb's code.

There is now one .different() method which takes an optional parameter of base_field, which can either be a subfield or an embedding.

comment:5 follow-up: Changed 11 months ago by cremona

Most of the code in the new method concerns computing the appropriate charpoly (absolute or relative, etc). Would it not be better to extend the functionality of the existing charpoly() methods first, then different() becomes very simple? For an element a of an absolute field all you need is a.charpoly() and for a relative extension either a.charpoly() for the relative one or a.absolute_charpoly() for the absolute one.

In fact, seeing as both those versoin of charpoly() exist already, just have methods a.different() and a.absolute_different() calling one or the other?

By the way, for some number field methods such as discriminant(), one is forced to use either absolute_discriminant() or relative_discriminant() always, to avoid errors. That should probably also be tru of charpoly) for consistency, and then similarly for different().

comment:6 Changed 11 months ago by git

  • Commit changed from 89957f6bbec3587ca1bf6e9ee657dd4b846e9fb2 to 6ac030c86bd016298b325117e13f6feef1eae0c6

Branch pushed to git repo; I updated commit sha1. New commits:

6ac030creplaced charpoly logic with a call to self.matrix like the other functions do, added absolute_different as a shortcut for basefield=QQ

comment:7 in reply to: ↑ 5 Changed 11 months ago by tscholl2

Replying to cremona:

Consistency is hard. The following are methods on class NumberFieldElement(FieldElement):

def trace(self, K=None): ...
def norm(self, K=None): ...
def absolute_norm(self): ...
def relative_norm(self): ...
def charpoly(self, var='x'): ...
def minpoly(self, var='x'): ...
def matrix(self, base=None): ...

Both absolute and relative number field elements also implement

def absolute_charpoly(...)
def absolute_minpoly(...)

There is no relative_charpoly or relative_minpoly as far as I can tell. None of these methods seem to be restricted like discriminant is for relative number fields.

Pretty much all of these methods just call self.matrix, with the exception that sometimes there is an optional argument to use Pari.

I thought the most consistent thing would be to make different and absolute_different like it is for charpoly and minpoly. Though I would want different to be more similar to trace or norm.

I also realized self.matrix was doing all the same logic I was trying to do, so that means the implementation is now just two lines now.

comment:8 follow-up: Changed 10 months ago by slelievre

  • Description modified (diff)
  • Milestone changed from sage-9.1 to sage-9.2
  • Reviewers set to John Cremona, Vincent Delecroix, Samuel Lelièvre

Some suggestions based on our documentation guidelines

  • prefer imperative "Return" to descriptive "Returns"
  • use :: before each example code block
  • pep8: spaces after commas and around + and -
  • add cross-links with .. SEEALSO::
-    def different(self,K=None):
+    def different(self, K=None):
         r"""
-        Returns the different of the element ``self`` with respect to the given
-        base field ``K``, which defaults ``self.parent().base_field()``.
+        Return the different of this element with respect to the given
+        base field.

         The different of an element `a` in an extension of number fields `L/K`
         is defined as `\mathrm{Diff}_{L/K}(a) = f'(a)` where `f` is the
         characteristic polynomial of `a` over `K`.

         INPUT:

         - ``K`` -- a subfield (embedding of a subfield) of the parent
-        number field ``K`` of ``self``. The default is ``K.base_field()``.
+          number field of ``self``. Default: ``None``, which will amount
+          to ``self.parent().base_field()``.

         EXAMPLES::

-            sage: K.<a> = NumberField(x^3-2)
+            sage: K.<a> = NumberField(x^3 - 2)
             sage: a.different()
             3*a^2
             sage: a.different(K=K)
             1

-        The optional argument ``K`` can be an embedding of a subfield
-        as shown here.
+        The optional argument ``K`` can be an embedding of a subfield::
 
             sage: K.<b> = NumberField(x^4 - 2)
             sage: (b^2).different()
             0
             sage: phi = K.base_field().embeddings(K)[0]
             sage: b.different(K=phi)
             4*b^3

-        Here we compute the relative and absolute different of an element
-        in a relative number field.
+        Compare the relative different and the absolute different
+        for an element in a relative number field::

             sage: K.<a> = NumberFieldTower([x^2 - 17, x^3 - 2])
             sage: a.different()
             2*a0
             sage: a.different(K=QQ)
             0
             sage: a.absolute_different()
             0

-        In this example, we can see that the different of the extension `\QQ(i)/\QQ`
-        is generated by the different of `i`.
+        Observe that for the field extension `\QQ(i)/\QQ`, the different of
+        the field extension is the ideal generated by the different of `i`::

-            sage: K.<c> = NumberField(x^2+1)
+            sage: K.<c> = NumberField(x^2 + 1)
             sage: K.different() == K.ideal(c.different())
             True
+
+        .. SEEALSO::
+
+            :meth:`absolute_different`
+            :meth:`sage.rings.number_field.number_field_rel.NumberField_relative.different`
         """
         f = self.matrix(base=K).charpoly().derivative().change_ring(self.parent())
         return f(self)
 
     def absolute_different(self):
         r"""
-        Returns the absolute different of the element ``self`` with respect to the
-        base field `\QQ`. See ``.different()`` for examples.
+        Return the absolute different of this element.
+
+        This means the different with respect to the base field `\QQ`.
+
+        EXAMPLE::
+
+            sage: K.<a> = NumberFieldTower([x^2 - 17, x^3 - 2])
+            sage: a.absolute_different()
+            0
+
+        .. SEEALSO::
+
+            :meth:`different`
         """
         return self.different(K=QQ)

comment:9 Changed 10 months ago by git

  • Commit changed from 6ac030c86bd016298b325117e13f6feef1eae0c6 to cb841def3d47c9b35cdbe875f28067476ad001ac

Branch pushed to git repo; I updated commit sha1. New commits:

cb841demore compliant documentation following reviewers comment

comment:10 in reply to: ↑ 8 Changed 10 months ago by tscholl2

Replying to slelievre:

Thanks for the suggestions, I amended the documentation accordingly.

comment:11 Changed 10 months ago by slelievre

Other reviewers, any more comments? Anything in the way of a positive review?

comment:12 Changed 10 months ago by cremona

  • Status changed from needs_review to positive_review

I'm a lot happier than when I first looked, now that the implementation is a one-liner and the docs are good.

Last edited 9 months ago by slelievre (previous) (diff)

comment:13 Changed 9 months ago by chapoton

  • Authors changed from caleb geiger, Travis Scholl to Caleb Geiger, Travis Scholl

comment:14 Changed 9 months ago by vbraun

  • Branch changed from u/tscholl2/24302 to cb841def3d47c9b35cdbe875f28067476ad001ac
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.