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
- 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
- Commit set to d00b70614ae504cec82bd75de9e1147204b25944
- Status changed from new to needs_review
comment:3 follow-up: ↓ 4 Changed 5 years ago by
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
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: ↓ 6 Changed 5 years ago by
But your question makes sense: why should it have changed?
comment:6 in reply to: ↑ 5 Changed 5 years ago by
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: ↓ 8 Changed 5 years ago by
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
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: ↓ 10 ↓ 11 Changed 5 years ago by
- 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
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
.
comment:11 in reply to: ↑ 9 Changed 5 years ago by
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
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
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
- 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
- Status changed from needs_review to positive_review
comment:16 Changed 5 years ago by
- Branch changed from u/jdemeyer/ticket/17166 to d00b70614ae504cec82bd75de9e1147204b25944
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Add coercion complex -> CC