sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.45k stars 482 forks source link

Error in comparing two Symbolic Ring elements #34235

Closed yuan-zhou closed 2 years ago

yuan-zhou commented 2 years ago

It seems a bug:

sage: bool(AA(sqrt(2))/pi == sqrt(2)/pi)
TypeError: ECL says: THROW: The catch MACSYMA-QUIT is undefined.

The following comparison is fine, though.

sage: bool(AA(sqrt(2)) == sqrt(2))
True

To fix this, we provide elements of AA and QQbar with a _maxima_init_ method that

CC: @mkoeppe @DaveWitteMorris @videlec @tscrim

Component: symbolics

Author: Matthias Koeppe

Branch/Commit: 58f4cd1

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/34235

mkoeppe commented 2 years ago
comment:2

Confirmed in 9.7.beta5

mkoeppe commented 2 years ago

Branch: u/mkoeppe/error_in_comparing_two_symbolic_ring_elements

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -10,3 +10,6 @@
 sage: bool(AA(sqrt(2)) == sqrt(2))
 True

+ +To fix this, we provide elements of AA and QQbar with a _maxima_init_ method that handles the easiest cases (rationals, square roots of rationals) and raises an error in all other cases. This is better than sending the unusable print representation of algebraic numbers (including ?) to Maxima. +

mkoeppe commented 2 years ago

New commits:

a627ce6src/sage/rings/qqbar.py (AlgebraicNumber_base._maxima_init_): New
mkoeppe commented 2 years ago

Commit: a627ce6

mkoeppe commented 2 years ago

Author: Matthias Koeppe

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from a627ce6 to 6204a76

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

6204a76src/sage/rings/qqbar.py (AlgebraicNumber_base._maxima_init_): Fixup
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 6204a76 to 3e560cf

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

3e560cfsrc/sage/rings/qqbar.py: Add doctest
mkoeppe commented 2 years ago
comment:7

Instead of defining _maxima_init_, perhaps it's better to define _symbolic_

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

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

58f4cd1src/sage/rings/qqbar.py (AlgebraicNumber_base._maxima_init_): Generalize using radical_expression
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 2 years ago

Changed commit from 3e560cf to 58f4cd1

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -11,5 +11,5 @@
 True

-To fix this, we provide elements of AA and QQbar with a _maxima_init_ method that handles the easiest cases (rationals, square roots of rationals) and raises an error in all other cases. This is better than sending the unusable print representation of algebraic numbers (including ?) to Maxima. +To fix this, we provide elements of AA and QQbar with a _maxima_init_ method that handles the case of elements for which Sage can find a radical expression and raises an error in all other cases. This is better than sending the unusable print representation of algebraic numbers (including ?) to Maxima.

mkoeppe commented 2 years ago

Description changed:

--- 
+++ 
@@ -11,5 +11,8 @@
 True

-To fix this, we provide elements of AA and QQbar with a _maxima_init_ method that handles the case of elements for which Sage can find a radical expression and raises an error in all other cases. This is better than sending the unusable print representation of algebraic numbers (including ?) to Maxima. +To fix this, we provide elements of AA and QQbar with a _maxima_init_ method that +- handles the case of elements for which Sage can find a radical expression and +- raises an error in all other cases. +This is better than sending the unusable print representation of algebraic numbers (including ?) to Maxima.

tscrim commented 2 years ago
comment:13

LGTM.

tscrim commented 2 years ago

Reviewer: Travis Scrimshaw

mkoeppe commented 2 years ago
comment:14

Thanks!

vbraun commented 2 years ago

Changed branch from u/mkoeppe/error_in_comparing_two_symbolic_ring_elements to 58f4cd1