Opened 5 years ago

Closed 5 years ago

#17166 closed enhancement (fixed)

Add coercion complex -> CC

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.4
Component: coercion Keywords:
Cc: robertwb Merged in:
Authors: Jeroen Demeyer Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: d00b706 (Commits) Commit: d00b70614ae504cec82bd75de9e1147204b25944
Dependencies: Stopgaps:

Description

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

Change History (16)

comment:1 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/17166
  • Created changed from 10/16/14 17:42:12 to 10/16/14 17:42:12
  • Modified changed from 10/16/14 17:42:12 to 10/16/14 17:42:12

comment:2 Changed 5 years ago by jdemeyer

  • Commit set to d00b70614ae504cec82bd75de9e1147204b25944
  • Status changed from new to needs_review

New commits:

d00b706Add coercion complex -> CC

comment:3 follow-up: Changed 5 years ago by 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? (I'm just wondering about breaking existing third-party code.)

comment:4 in reply to: ↑ 3 Changed 5 years ago by jdemeyer

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.

comment:5 follow-up: Changed 5 years ago by jdemeyer

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

comment:6 in reply to: ↑ 5 Changed 5 years ago by jdemeyer

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.

comment:7 follow-up: Changed 5 years ago by vbraun

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.

comment:8 in reply to: ↑ 7 Changed 5 years ago by jdemeyer

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.

comment:9 follow-ups: Changed 5 years ago by vbraun

  • Cc robertwb added

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.

comment:10 in reply to: ↑ 9 Changed 5 years ago by jdemeyer

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.

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:11 in reply to: ↑ 9 Changed 5 years ago by jdemeyer

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

comment:12 Changed 5 years ago by robertwb

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

comment:13 Changed 5 years ago by jdemeyer

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.

comment:14 Changed 5 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

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.

comment:15 Changed 5 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:16 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/17166 to d00b70614ae504cec82bd75de9e1147204b25944
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.