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

range uses TwicePrecision when possible (part 2) #44528

Merged
merged 12 commits into from
Mar 11, 2022

Conversation

jipolanco
Copy link
Contributor

Follow-up of #44313. See also discussion in #43059.

Fixes the return type of:

  • range(start::IEEEFloat; step::Real, length)
  • range(; stop::IEEEFloat, step::Real, length)
  • range(; stop::Real, step::IEEEFloat, length)

making sure that TwicePrecision is always used in these cases.

base/twiceprecision.jl Outdated Show resolved Hide resolved
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some tests for mixed-IEEEFloat calls, since I think there may be some ambiguity errors there:

julia> range(0.0f0, step=1.0e0, length=10)
0.0:1.0:9.0

julia> typeof(ans)
StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}

julia> range(0.0e0, step=1.0f0, length=10)
0.0f0:1.0f0:9.0f0

julia> typeof(ans)
StepRangeLen{Float32, Float64, Float64, Int64}

base/twiceprecision.jl Outdated Show resolved Hide resolved
@jipolanco
Copy link
Contributor Author

Now ambiguities should be resolved, unless I missed something. I added a bunch of different tests to check this.

TwicePrecision is also now limited to real-valued ranges, so that e.g. complex ranges work correctly again.

For now all of this is done by defining several variants of range_start_step_length and range_step_stop_length. I'm sure there's a more elegant way of doing this, but this solution seems to work and is quite simple to understand.

jipolanco and others added 8 commits March 10, 2022 08:16
Fixes the return type of `range(start::IEEEFloat; step, length)` when
`step` is *not* an `IEEEFloat` (e.g. when it's an integer).
- check complex ranges
- check for ambiguities
@dkarrasch dkarrasch added the domain:ranges Everything AbstractRange label Mar 10, 2022
Copy link
Sponsor Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Thank you

base/twiceprecision.jl Outdated Show resolved Hide resolved
base/twiceprecision.jl Outdated Show resolved Hide resolved
base/twiceprecision.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC mentioned this pull request Mar 11, 2022
47 tasks
@KristofferC KristofferC merged commit 45ab664 into JuliaLang:master Mar 11, 2022
KristofferC pushed a commit that referenced this pull request Mar 12, 2022
@jipolanco jipolanco deleted the jip/ranges-intstep branch March 12, 2022 11:52
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ranges Everything AbstractRange
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants