#18220 closed defect (fixed)
disallow non exact fields for the 'field' backend
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage7.4 
Component:  geometry  Keywords:  bug, days84 
Cc:  chapoton, jipilab, mkoeppe  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  c28fcf0 (Commits)  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
The 'field' backend does not properly support non exact fields
sage: omega = 2*RR.pi() / 5 sage: verts = [((i*omega).sin(), (i*omega).cos()) for i in range(5)] sage: verts sage: Polyhedron(vertices=verts, base_ring=RR) Traceback (most recent call last): ... AssertionError:
or
sage: Q = polytopes.icosahedron() sage: Q = polytopes.icosahedron(); Q A 3dimensional polyhedron in (Number Field in sqrt5 with defining polynomial x^2  5)^3 defined as the convex hull of 12 vertices sage: Q_RR = Polyhedron(vertices = [n(v.vector(),digits=10) for v in Q.vertices()]) sage: Q_RR A 3dimensional polyhedron in (Real Field with 37 bits of precision)^3 defined as the convex hull of 7 vertices sage: Q_RR = Polyhedron(vertices = [n(v.vector(),digits=30) for v in Q.vertices()]) sage: Q_RR A 3dimensional polyhedron in (Real Field with 103 bits of precision)^3 defined as the convex hull of 1 vertex and 3 rays
We simply raise an error if somebody want to try this.
Change History (22)
comment:1 Changed 5 years ago by
 Keywords bug added
comment:2 Changed 4 years ago by
comment:3 Changed 4 years ago by
 Cc chapoton added
 Milestone changed from sage6.7 to sage7.4
comment:4 Changed 3 years ago by
 Cc jipilab added
 Description modified (diff)
comment:5 Changed 3 years ago by
 Branch set to u/vdelecroix/18220
 Commit set to c28fcf05115ba98f832e74a94eb518bb53117d2a
 Description modified (diff)
 Status changed from new to needs_review
 Summary changed from Bug when creating a polyhedron with coefficients in RR to disallow non exact fields for the 'field' backend
New commits:
c28fcf0  18220: raise an error for non exact fields

comment:6 Changed 3 years ago by
 Keywords days84 added
comment:7 Changed 3 years ago by
 Cc mkoeppe added
comment:8 Changed 3 years ago by
The tests pass on my machine with 7.6.beta6. The documentation looks fine as well. No idea what the quasar bot is up to.
The I believe it is a good idea to raise an error for such a behaviour.
Matthias, any thoughts? Does that look good for you?
comment:9 Changed 3 years ago by
I agree with disabling it for floating point, but I'm not sure about disabling it for the SR as well. Maybe there are some people who want to work with polytopes with transcendental coordinates?
comment:10 Changed 3 years ago by
I would like to make computations with trascendental coordinates. However, the symbolic ring is not the proper way to do it. I am very much against special casing SR anywhere. SR has its own logic that does not fit at all with the Sage logic of parents and coercion. Moreover, comparison is completely broken
sage: bool(pi == 245850922/78256779) True
comment:11 Changed 3 years ago by
 Reviewers set to JeanPhilippe Labbé
 Status changed from needs_review to positive_review
Then, let's restrict the usage of the symbolic ring with polyhedron.
If it ever happens that there is a use case of the symbolic ring, we may consider adapting it properly then.
comment:12 Changed 3 years ago by
 Status changed from positive_review to needs_info
WAIT!!! Are you proposing to drop support for RR/RDF and other floating points?! With all the bugs it is still useful to be able to operate with them, if you think it is too dangerous, how about throwing a warning?
comment:13 Changed 3 years ago by
As mentioned in the title description, it only concerns the "field" backend that is very much broken for non exact fields. Other backends still support RDF
as it used to be.
comment:14 followup: ↓ 15 Changed 3 years ago by
 Status changed from needs_info to positive_review
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
comment:15 in reply to: ↑ 14 ; followup: ↓ 16 Changed 3 years ago by
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
No worries!
It is true that the name 'field' is confusing...
comment:16 in reply to: ↑ 15 ; followup: ↓ 17 Changed 3 years ago by
Replying to jipilab:
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
No worries!
It is true that the name 'field' is confusing...
What about changing it to 'generic'?
comment:17 in reply to: ↑ 16 ; followup: ↓ 18 Changed 3 years ago by
Replying to vdelecroix:
Replying to jipilab:
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
No worries!
It is true that the name 'field' is confusing...
What about changing it to 'generic'?
or 'generic_exact', or 'sage_toy', or ...
comment:18 in reply to: ↑ 17 ; followup: ↓ 19 Changed 3 years ago by
Replying to vdelecroix:
Replying to vdelecroix:
Replying to jipilab:
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
No worries!
It is true that the name 'field' is confusing...
What about changing it to 'generic'?
or 'generic_exact', or 'sage_toy', or ...
In this setup I would not call the backend generic, because it does not provide the principal reason to use it: irrational exact values.
What about irrational_exact
?
comment:19 in reply to: ↑ 18 ; followup: ↓ 20 Changed 3 years ago by
Replying to jipilab:
Replying to vdelecroix:
Replying to vdelecroix:
Replying to jipilab:
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
No worries!
It is true that the name 'field' is confusing...
What about changing it to 'generic'?
or 'generic_exact', or 'sage_toy', or ...
In this setup I would not call the backend generic, because it does not provide the principal reason to use it: irrational exact values.
What about
irrational_exact
?
It is even more confusing since this backend can deal with rationals... generic_exact
?
comment:20 in reply to: ↑ 19 Changed 3 years ago by
Replying to vdelecroix:
Replying to jipilab:
Replying to vdelecroix:
Replying to vdelecroix:
Replying to jipilab:
OK, sorry for the false alarm (it was an end of a tough day yesterday ;)). I guess I got confused by the strange name for a backend as well...
No worries!
It is true that the name 'field' is confusing...
What about changing it to 'generic'?
or 'generic_exact', or 'sage_toy', or ...
In this setup I would not call the backend generic, because it does not provide the principal reason to use it: irrational exact values.
What about
irrational_exact
?It is even more confusing since this backend can deal with rationals...
generic_exact
?
Right... hmm.
+1 for generic_exact
.
;)
comment:21 Changed 3 years ago by
 Branch changed from u/vdelecroix/18220 to c28fcf05115ba98f832e74a94eb518bb53117d2a
 Resolution set to fixed
 Status changed from positive_review to closed
comment:22 Changed 3 years ago by
 Commit c28fcf05115ba98f832e74a94eb518bb53117d2a deleted
I just saw that in the documentation of version 7.6.rc0, the doc file of the backend field
is called "the python backend".
Maybe we should create a ticket about renaming the backend.
Another bug for RDF polyhedra: #21270