Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add coercion complex -> CC #17166

Closed
jdemeyer opened this issue Oct 16, 2014 · 17 comments
Closed

Add coercion complex -> CC #17166

jdemeyer opened this issue Oct 16, 2014 · 17 comments

Comments

@jdemeyer
Copy link

This works fine:

sage: CC(complex('13.8+6.2j'))
13.8000000000000 + 6.20000000000000*I
sage: CDF(complex('13.8+6.2j'))
13.8 + 6.2*I

However, it is a conversion while it should be a coercion.

sage: CC.has_coerce_map_from(complex)
False

This is inconsistent with

sage: CC.has_coerce_map_from(float)
True

CC: @robertwb

Component: coercion

Author: Jeroen Demeyer

Branch/Commit: d00b706

Reviewer: Travis Scrimshaw

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

@jdemeyer jdemeyer added this to the sage-6.4 milestone Oct 16, 2014
@jdemeyer
Copy link
Author

Branch: u/jdemeyer/ticket/17166

@jdemeyer
Copy link
Author

Commit: d00b706

@jdemeyer
Copy link
Author

New commits:

d00b706Add coercion complex -> CC

@kcrisman
Copy link
Member

comment:3

What did the old doctest

a=matrix([[1, 1e-4r, 1+1e-100jr], [1e-8+3j, 0, 1e-58r]])

do with the changes in question if you didn't change the ring? (I'm just wondering about breaking existing third-party code.)

@jdemeyer
Copy link
Author

comment:4

Replying to @kcrisman:

What did the old doctest

a=matrix([[1, 1e-4r, 1+1e-100jr], [1e-8+3j, 0, 1e-58r]])

do with the changes in question if you didn't change the ring?

Return a matrix over CC instead of CDF.

@jdemeyer
Copy link
Author

comment:5

But your question makes sense: why should it have changed?

@jdemeyer
Copy link
Author

comment:6

Replying to @jdemeyer:

But your question makes sense: why should it have changed?

The change makes sense because the parent of the entry 1e-8+3j is CC. Therefore, it is logical that the matrix is defined over CC.

@vbraun
Copy link
Member

vbraun commented Oct 18, 2014

comment:7

IMHO the coercion should be complex -> CDF -> CC and not complex -> CC -> CDF. Pretty much the only difference is that the matrix would default to CDF, but since matrices over CDF are much more useful than matrices over CC this is probably what the user prefers.

@jdemeyer
Copy link
Author

comment:8

Replying to @vbraun:

IMHO the coercion should be complex -> CDF -> CC and not complex -> CC -> CDF.

Coercion goes both ways in this case (CDF -> CC and CC -> CDF), so the result is the same really. Interestingly, this has the following consequence:

sage: Sequence([CDF(1), CC(1)]).parent()
Category of sequences in Complex Double Field
sage: Sequence([CC(1), CDF(1)]).parent()
Category of sequences in Complex Field with 53 bits of precision

Let me know what you think, if you think the coercion should be added to ComplexDoubleField._coerce_map_from(), I could do that.

@vbraun
Copy link
Member

vbraun commented Oct 19, 2014

comment:9

I also noticed that we have coercions both way CDF <-> CC. This sounds a bit wonky, IMHO it shoud be ComplexField(>=53) -> CDF -> ComplexField(<53). Though this has been around for a long time, perhaps there is a deeper reason for that? Maybe Robert knows? Possibly stuff for another ticket, though.

What I originally meant was just that Python complex should coerce to CDF and not CC.

@jdemeyer
Copy link
Author

comment:10

Replying to @vbraun:

What I originally meant was just that Python complex should coerce to CDF and not CC.

Having a coercion complex -> CDF and CDF -> CC but not complex -> CC sounds strange to me (isn't coercion supposed to be transitive?). Moreover, that would be inconsistent with

sage: RR.has_coerce_map_from(float)
True
sage: RDF.has_coerce_map_from(float)
True

Whatever the outcome of this ticket is, the behaviour of complex / CDF / CC should be the same as float / RDF / RR.

@jdemeyer
Copy link
Author

comment:11

Replying to @vbraun:

IMHO it shoud be ComplexField(>=53) -> CDF -> ComplexField(<53).

I would prefer CDF -> ComplexField(53) simply because there are numbers in CC which cannot be represented in CDF:

sage: CC(1e1000)
1.00000000000000e1000
sage: CDF(1e1000)
+infinity

@robertwb
Copy link
Contributor

comment:12

Yes, it should be ComplexField(>53) -> CDF -> ComplexField(<=53). We should also have complex -> CDF. This does induce a coercion complex -> ComplexField(<=53).

@jdemeyer
Copy link
Author

comment:13

Can this ticket please be reviewed as just making complex/CC consistent with float/RR? Further discussions about changing coercion between CC and CDF can still go to another ticket.

@tscrim
Copy link
Collaborator

tscrim commented Nov 4, 2014

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Nov 4, 2014

comment:14

I'm okay with pushing the issues with the other coercions to another ticket, and the current branch LGTM. So I'm setting a positive review.

@vbraun
Copy link
Member

vbraun commented Nov 15, 2014

Changed branch from u/jdemeyer/ticket/17166 to d00b706

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants