Opened 6 years ago

Closed 6 years ago

# abs for number field element

Reported by: Owned by: Vincent Delecroix major sage-7.4 number fields Matthias Köppe Matthias Koeppe Vincent Delecroix N/A 15d160e 15d160e2ec536e988494080f187e164f26ddd291

For a number field element `a`, `abs(a)` currently returns a floating point real number

```sage: K.<cbrt2> = NumberField(x^3 - 2, 'a', embedding=1.26)
sage: abs(cbrt2)
1.25992104989487
sage: parent(_)
Real Field with 53 bits of precision
```

If a coercion embedding is defined with value in `RR`, the absolute value can be defined internally as it is the case for `AA`

```sage: abs(AA(2).sqrt() - AA(2))
0.5857864376269049?
```

```sage: K.<sqrt2> = NumberField(x^2 - 2, 'a', embedding=1.41)
sage: abs(sqrt2)
sqrt2
```

We propose to change the behavior for embedded number fields. Namely make `abs` an internal operation.

As a (minor) consequence of the current behavior, the inequalities from `sage.geometry.polyhedron.representation.Inequality` gets badly displayed

```sage: K.<cbrt2> = NumberField(x^3 - 2, 'a', embedding=1.26)
sage: P = Polyhedron(vertices=[(1,1,cbrt2),(cbrt2,1,1)])
sage: P.inequalities()
(An inequality (-cbrt2^2 - cbrt2 - 1, 0, 0) x + 4.84732210186307 >= 0,
An inequality (cbrt2^2 + cbrt2 + 1, 0, 0) x - 3.84732210186307 >= 0)
```

### comment:1 Changed 6 years ago by Matthias Köppe

Description: modified (diff)

### comment:2 Changed 6 years ago by Matthias Köppe

Branch: → u/mkoeppe/abs_for_number_field_element

### comment:3 Changed 6 years ago by git

Commit: → 33e5f4cc2b7b4312cac49d5d9c8c07b876a212de

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

 ​33e5f4c `Add test for #21105`

### comment:4 Changed 6 years ago by Matthias Köppe

Authors: → Matthias Koeppe new → needs_review

### comment:5 Changed 6 years ago by Vincent Delecroix

It is weird to have `abs(a)` and `a.abs()` behaving differently, don't you think?

### comment:6 follow-up:  13 Changed 6 years ago by Matthias Köppe

OK, I'll change `a.abs()` too.

### comment:7 Changed 6 years ago by Matthias Köppe

By the way, should perhaps the print representation of a number field indicate whether it's embedded or not?

### comment:8 follow-up:  9 Changed 6 years ago by Vincent Delecroix

That's indeed an issue. But it is hard to solve since you are interested at the same time in both the algebraic expression and the embedded value.

More generally, instead of having if/else in many methods I think that a better solution would be to have a dedicated class for real embedded element (whose semantic would be to follow standard method of real numbers, possibly having methods like `.cos()`, `.exp()` and such). But that is another question that will not be solved by the ticket.

### comment:9 in reply to:  8 ; follow-up:  10 Changed 6 years ago by Matthias Köppe

That's indeed an issue. But it is hard to solve since you are interested at the same time in both the algebraic expression and the embedded value.

True, but I meant the print representation of the field itself (the parent), not an element.

### comment:10 in reply to:  9 ; follow-up:  12 Changed 6 years ago by Vincent Delecroix

That's indeed an issue. But it is hard to solve since you are interested at the same time in both the algebraic expression and the embedded value.

True, but I meant the print representation of the field itself (the parent), not an element.

+1. Put me in cc if you open a ticket.

### comment:11 Changed 6 years ago by git

Commit: 33e5f4cc2b7b4312cac49d5d9c8c07b876a212de → 0c31b4956e5660faafa242bc089b7044e39db65e

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

 ​0c31b49 `Trac 21105: Make x.abs() behave the same as abs(x) for real embedded number field elements`

### comment:12 in reply to:  10 Changed 6 years ago by Matthias Köppe

That's indeed an issue. But it is hard to solve since you are interested at the same time in both the algebraic expression and the embedded value.

True, but I meant the print representation of the field itself (the parent), not an element.

+1. Put me in cc if you open a ticket.

See #21161.

### comment:13 in reply to:  6 Changed 6 years ago by Matthias Köppe

OK, I'll change `a.abs()` too.

Done.

### comment:14 Changed 6 years ago by Vincent Delecroix

This is missing indentation

```Check that for fields with real coercion embeddings, absolute
values are in the same field (:trac:`21105`)::

sage: x = polygen(ZZ)      <---- test should be indented
```

### comment:15 Changed 6 years ago by git

Commit: 0c31b4956e5660faafa242bc089b7044e39db65e → ba1fbec6a975d37711181292157c4866ca3c148e

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

 ​ba1fbec `Fix indentation of examples`

### comment:16 Changed 6 years ago by Matthias Köppe

Reviewers: → Vincent Delecroix

### comment:17 follow-up:  19 Changed 6 years ago by Vincent Delecroix

After

```            sage: x = polygen(ZZ)
sage: f = x^3 - x - 1
sage: K.<b> = NumberField(f, embedding=1.3)
sage: b.abs()
b
```

you should also test the output with arguments (both `i` and `prec`).

Last edited 6 years ago by Vincent Delecroix (previous) (diff)

### comment:18 Changed 6 years ago by git

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

 ​586ce66 `NumberFieldElement.abs: Add doctest` ​f7b105f `real embedded numberfield abs: Return complex if prec is given`

### comment:19 in reply to:  17 Changed 6 years ago by Matthias Köppe

you should also test the output with arguments (both `i` and `prec`).

Done. I've also changed what happens when `prec` is provided. I think it makes more sense now.

### comment:20 Changed 6 years ago by Vincent Delecroix

If `prec=None` and there is no real embedded, I would actually return an element of `AA` (the real algebraic field). But it becomes beyond the scope of the ticket... (and breaks even more the previous behavior).

### comment:21 Changed 6 years ago by Matthias Köppe

Yes, I guess that would make sense; but I agree it's too much for this ticket.

By the way, why does it use complex fields instead of real fields for the result?

### comment:22 follow-up:  24 Changed 6 years ago by Vincent Delecroix

It does not: the method uses complex field for the embedding. But the result of `.abs()` on a complex number is real

```sage: CC(0,1).abs().parent()
Real Field with 53 bits of precision
```

### comment:23 Changed 6 years ago by git

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

 ​c7634ff `NumberFieldElement.abs: Clarify return type`

### comment:24 in reply to:  22 Changed 6 years ago by Matthias Köppe

It does not: the method uses complex field for the embedding. But the result of `.abs()` on a complex number is real

Ah, thanks. I've reworded the docstring to clarify.

### comment:25 follow-up:  27 Changed 6 years ago by Vincent Delecroix

I don't understand this sentence from the doc of `abs`

```If ``prec`` is ``None`` or 53, then the complex double field is
used; otherwise the arbitrary precision (but slow) complex
field is used.  The result is in the corresponding real field.
```

The "complex double field" refers to I guess "CDF" but which is actually not used! I would rather remove that sentence and explain that the default precision is `53`.

### comment:26 Changed 6 years ago by git

Commit: c7634ff5562f2cc59288f0986c8538a16d37f39e → 15d160e2ec536e988494080f187e164f26ddd291

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

 ​15d160e `NumberFieldElement.abs: Clarify that CDF is not used`

### comment:27 in reply to:  25 Changed 6 years ago by Matthias Köppe

I don't understand this sentence from the doc of `abs`

```If ``prec`` is ``None`` or 53, then the complex double field is
used; otherwise the arbitrary precision (but slow) complex
field is used.  The result is in the corresponding real field.
```

The "complex double field" refers to I guess "CDF" but which is actually not used! I would rather remove that sentence and explain that the default precision is `53`.

Thanks for catching this.

### comment:28 Changed 6 years ago by Vincent Delecroix

Milestone: sage-7.3 → sage-7.4 needs_review → positive_review

Thanks for the fix!

### comment:29 Changed 6 years ago by Volker Braun

Branch: u/mkoeppe/abs_for_number_field_element → 15d160e2ec536e988494080f187e164f26ddd291 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.