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

Promotion of Irrational{s} with Complex{Irrational{s}} gives StackOverflowError #51001

Closed
jmbyrne opened this issue Aug 22, 2023 · 7 comments · Fixed by #55870
Closed

Promotion of Irrational{s} with Complex{Irrational{s}} gives StackOverflowError #51001

jmbyrne opened this issue Aug 22, 2023 · 7 comments · Fixed by #55870
Labels
bug Indicates an unexpected problem or unintended behavior complex Complex numbers

Comments

@jmbyrne
Copy link

jmbyrne commented Aug 22, 2023

image

The promote_rule with an Irrational{s} type and a Complex{Irrational{s}} type (where s is the same Symbol) gives a StackOverflowError. The intended type is probably Complex{Irrational{s}}, as demonstrated by swapping the arguments:

image

If the parameters are different (e.g. Irrational{:ℯ} with Complex{Irrational{:π}}), then there is no issue as it promotes to ComplexF64.


Julia version 1.9.2 installed on Windows 11 with the Julia installer https://julialang.org/downloads/

@Seelengrab Seelengrab added the complex Complex numbers label Aug 22, 2023
@Seelengrab
Copy link
Contributor

Seelengrab commented Aug 22, 2023

A bit orthogonal, but please note that Julia is not a CAS - not everything is expected to preserve irrationals, or combinations thereof. This looks like it should be fixed though, as simply swapping the order of arguments already works.

@jmbyrne
Copy link
Author

jmbyrne commented Aug 22, 2023

I agree that since Irrational types are singleton types it would be very rare to actually come across this. However, it is possible, for instance promote(π, complex(π, π)) causes the same error. Not sure why you'd use complex(π, π), but there might be a good reason!

For a bit of added context, I was trying to promote every possible pair of numeric types to generate a diagram, but this was stopping it. Since you mentioned swapping the arguments, there is some more strange behaviour related to that, e.g.:
image

I don't know whether this is actually a problem, and if it is, whether it should be its own issue anyway. I'll let someone else be the judge of that.

@bvdmitri
Copy link
Contributor

I think you are supposed to use promote_type instead, which uses the promote_rule under the hood but also ensures symmetry.

@jmbyrne
Copy link
Author

jmbyrne commented Aug 22, 2023

You're right, I missed that. The original issue still stands though, that breaks with either promote_type or promote_rule.

@nsajko nsajko added the bug Indicates an unexpected problem or unintended behavior label Sep 25, 2024
@nsajko
Copy link
Contributor

nsajko commented Sep 25, 2024

Faulty rule:

promote_rule(::Type{S}, ::Type{T}) where {S<:AbstractIrrational,T<:Number} = promote_type(promote_type(S, real(T)), T)

xref #55866

@mbauman
Copy link
Member

mbauman commented Sep 25, 2024

Is this the same as #13193?

@nsajko
Copy link
Contributor

nsajko commented Sep 25, 2024

No, this one should be easier to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior complex Complex numbers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants