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

setrounding regression for 0.5.0-rc1 #17926

Closed
omus opened this issue Aug 9, 2016 · 17 comments
Closed

setrounding regression for 0.5.0-rc1 #17926

omus opened this issue Aug 9, 2016 · 17 comments
Assignees
Labels
upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@omus
Copy link
Member

omus commented Aug 9, 2016

Taken from a rounding mode example in the manual:

Julia 0.4.6

julia> with_rounding(Float64,RoundDown) do
           1.1 + 0.1
       end
1.2

julia> 1.1 + 0.1
1.2000000000000002

Julia 0.5.0-rc1

julia> setrounding(Float64,RoundDown) do
           1.1 + 0.1
       end
1.2000000000000002

julia> 1.1 + 0.1
1.2000000000000002
@omus
Copy link
Member Author

omus commented Aug 9, 2016

Looks to be that the example function is being optimized:

julia> x = 0.1
0.1

julia> setrounding(Float64,RoundDown) do
           1.1 + x
       end
1.2

@tkelman tkelman added regression Regression in behavior compared to a previous version and removed regression Regression in behavior compared to a previous version labels Aug 9, 2016
@Keno
Copy link
Member

Keno commented Aug 10, 2016

I suspect this is LLVM inlining and constant folding. LLVM doesn't model rounding modes, so there is very little we can do.

@omus
Copy link
Member Author

omus commented Aug 10, 2016

Makes sense. I guess the best course of action would be to update the manual to have a more complicated but working example.

@simonbyrne
Copy link
Contributor

That's not good: previously we relied on the fact that anonymous functions weren't really optimised. Now that they are, I'm not sure what we can do until LLVM get around to allowing rounding mode changes (I wouldn't hold your breath...).

We should probably stick some warnings in there, and mark it as an experimental feature.

@yuyichao
Copy link
Contributor

IIUC constant folding are usually done at default rounding mode. I don't think it's possible to know the right rounding mode anyway unless we disable all constant folding for floating point numbers since the rounding mode is a runtime setting.

@simonbyrne
Copy link
Contributor

simonbyrne commented Aug 10, 2016

Could we have a meta tag that would disable constant folding in a particular region?

@yuyichao
Copy link
Contributor

@noinline

@simonbyrne
Copy link
Contributor

Doesn't that just disable constant folding at definition time? How could I use it in the above example?

@tkelman tkelman added the upstream The issue is with an upstream dependency, e.g. LLVM label Aug 10, 2016
@yuyichao
Copy link
Contributor

@noinline f() = 1.0; f() + 1.1

@yuyichao
Copy link
Contributor

I just mean that @noinline is the only reliable optimization barrier we have and is likely to stay for some time.

@simonbyrne
Copy link
Contributor

simonbyrne commented Aug 10, 2016

So one option is that we could change it to a macro which pulls out all the literals into a @noinline call, i.e.

@setrounding Float64 RoundDown 1.1 + 0.1

transforms to

@noinline literals() = (1.1,0.1)
old = rounding(Float64)
setrounding(Float64, RoundDown)
a,b = literals()
a+b
setrounding(Float64, old)

@simonbyrne
Copy link
Contributor

Although that won't work with consts...

@dpsanders
Copy link
Contributor

Ouch. This sounds like it will seriously break ValidatedNumerics.jl.
Why did this not get picked up by the Julia test suite?

@KristofferC
Copy link
Member

How did it not get picked up by your test suite? :P

@dpsanders
Copy link
Contributor

dpsanders commented Aug 11, 2016

Apparently because we never have literals; everything that gets rounded comes out of an Interval object.

@tkelman
Copy link
Contributor

tkelman commented Aug 11, 2016

I guess our rounding-mode tests aren't as comprehensive as they could be? That doc example was also not a doctest, but if it had been we likely would have noticed sooner.

simonbyrne added a commit that referenced this issue Aug 16, 2016
Fixes the doc problem mentioned in #17926, but not the underlying problem.
simonbyrne added a commit that referenced this issue Aug 16, 2016
Fixes the doc problem mentioned in #17926, but not the underlying problem.
tkelman pushed a commit that referenced this issue Aug 20, 2016
Fixes the doc problem mentioned in #17926, but not the underlying problem.

(cherry picked from commit f6e9bdf)
ref #18052

update docs, use new markdown syntax

(cherry picked from commit 28eeed8)

wrap help text

(cherry picked from commit 1bd4c70)
mfasi pushed a commit to mfasi/julia that referenced this issue Sep 5, 2016
Fixes the doc problem mentioned in JuliaLang#17926, but not the underlying problem.
@simonbyrne
Copy link
Contributor

setrounding has now been deprecated: #27166

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

No branches or pull requests

7 participants