Opened 10 years ago

Closed 10 years ago

# Numerical approximation of an algebraic number raises a ValueError

Reported by: Owned by: slabbe tbd major sage-4.7 numerical numerical_approx, AlgebraicNumber sage-4.7.alpha5 Simon Spicer Rob Beezer N/A

### Description

Numerical approximation works for complex numbers:

```sage: n(1 + I)
1.00000000000000 + 1.00000000000000*I
sage: (1 + I).n()
1.00000000000000 + 1.00000000000000*I
```

but not for algebraic numbers:

```sage: m = matrix(3, [3,1,6,5,2,9,7,3,13])
sage: E = m.eigenvalues()
sage: E
[18.16815365088822?, -0.08407682544410650? - 0.2190261484802906?*I, -0.08407682544410650? + 0.2190261484802906?*I]
sage: map(type, E)
[<class 'sage.rings.qqbar.AlgebraicNumber'>, <class 'sage.rings.qqbar.AlgebraicNumber'>, <class 'sage.rings.qqbar.AlgebraicNumber'>]
sage: map(n, E)
Traceback (most recent call last):
...
ValueError: Cannot coerce algebraic number with non-zero imaginary part to algebraic real
```

### comment:1 Changed 10 years ago by spice

• Authors set to Simon Spicer
• Component changed from PLEASE CHANGE to numerical
• Status changed from new to needs_review

The patch provides a short and sweet fix to the above problem. The issue arises in sage/misc/functional.py's numerical_approx() function:

```if not (is_ComplexNumber(x) or is_ComplexDoubleElement(x)):
try:
return sage.rings.real_mpfr.RealField(prec)(x)
except TypeError:
pass
return sage.rings.complex_field.ComplexField(prec)(x)
```

Attempting to call RealField?() on a complex AlgebraicNumber? raises a ValueError? and not a TypeError?, so the exception is not caught. Changing the except clause to catch all errors fixes this:

```if not (is_ComplexNumber(x) or is_ComplexDoubleElement(x)):
try:
return sage.rings.real_mpfr.RealField(prec)(x)
# Trac 10761: now catches all errors (instead of just
# a TypeError), since calling RealField on AlgebraicNumbers
# can raise a ValueError
except:
pass
return sage.rings.complex_field.ComplexField(prec)(x)
```

### comment:2 Changed 10 years ago by rbeezer

Hi Simon,

This will be very useful, thanks for digging it up. Three comments:

1. `type(E[1])` would be more accurate (and readable) as `E[1].parent()`, more Sage-like.
1. I write lots of "naked" except clauses, which I think is a bad practice. Is this a place where could just add `", Value Error"` after the `TypeError`?
1. Style Points: I think three lines of source comments for this fix is more than you would normally see. With the Trac number in the docstring, and a patch on Trac, you don't need to say so much. With one line here, it'd warn anybody away from messing with it.

Passes all long tests right now.

Rob

### comment:3 Changed 10 years ago by spice

Hi Rob

On the issues you've raised:

1) `type(E[1])` changed to `E[1].parent()` in the example as per your suggestion.

2) Regarding the naked except clause, I tried to add a catch of `ValueError`, but the function was still breaking. I wasn't able to figure out why, which is why I defaulted to catching everything. I figured this was okay, though, since this was just one part of a multistep coercion attempt - and thus errors are to be expected. Thoughts?

3) Inline comment streamlined to a single line.

Simon

### comment:4 Changed 10 years ago by rbeezer

• Reviewers set to Rob Beezer
• Status changed from needs_review to positive_review

Simon,

Looks real good. If the precise error list does not do the trick, then I think leaving it empty is fine.

Applies and builds, passes tests, docs build and look good. Positive review.

Rob

### comment:5 follow-up: ↓ 6 Changed 10 years ago by kini

This works for me...

• ## sage/misc/functional.py

`diff -r 361a4ad7d52c -r 87dedc409966 sage/misc/functional.py`
 a if not (is_ComplexNumber(x) or is_ComplexDoubleElement(x)): try: return sage.rings.real_mpfr.RealField(prec)(x) except TypeError: except (TypeError, ValueError): pass return sage.rings.complex_field.ComplexField(prec)(x)

I think it would be cleaner than just catching all exceptions. Simon, what exactly was wrong with catching ValueErrors??

### comment:6 in reply to: ↑ 5 Changed 10 years ago by slabbe

Simon, what exactly was wrong with catching ValueErrors???

I was going to ask the same, but kini beated me.

I also was going to suggest this if the tuple didn't work :

```except TypeError:
pass
except ValueError:
pass
```

but I think what kini proposes is best. No?

### comment:7 Changed 10 years ago by kini

Did you do something like this?

```except TypeError, ValueError:
```

This will attempt to catch TypeError exceptions and assign them to the variable name ValueError. It's a deprecated behavior from old versions of Python.

### Changed 10 years ago by spice

Replaces existing patch.

### comment:8 Changed 10 years ago by spice

• Status changed from positive_review to needs_work

Ah. n00b error - I forgot to add parentheses. It works now.

Simon

### comment:9 Changed 10 years ago by spice

• Status changed from needs_work to needs_review

### comment:10 Changed 10 years ago by kini

• Status changed from needs_review to positive_review