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

Solves some type instability issues, or diminishes its effects #92

Closed
wants to merge 2 commits into from

Conversation

lbenet
Copy link
Member

@lbenet lbenet commented Mar 11, 2016

Aims to solve, or at least diminish the consequences noticed in #91

@lbenet
Copy link
Member Author

lbenet commented Mar 11, 2016

I tried to solve this by annotating the expected type in different places.

@lbenet
Copy link
Member Author

lbenet commented Mar 11, 2016

I don't like the solution (type annotations) either, but at least solves the issue partially. I agree that not being too careful about what we construct, leads to this problem. I simply provided this solution, because it makes my computations nearly a factor 2 faster. (I still need to make other improvements.)

@lbenet
Copy link
Member Author

lbenet commented Mar 11, 2016

Tests actually work fine in Julia v0.5, but take 10x more!

The point is that some uses of these function/macro involve a function
"as an argument", which is known to be bad for performance.
@lbenet
Copy link
Member Author

lbenet commented Mar 15, 2016

Part of the problem of loosing type stability is related to the fact that some macros/functions, in a way, pass a function as an argument. This happens often when we are changing the rounding mode.

I just pushed a new commit (that only deals with the problematic functions in intervals/arithmetic.jl, i.e., fma, mig and diam) which yields type-stable code, which is actually faster than what I did previously.

@dpsanders
Copy link
Member

Passing functions as arguments is no longer slow in 0.5 since JuliaLang/julia#13412, so I'm not sure those changes are necessary.

@lbenet
Copy link
Member Author

lbenet commented Mar 15, 2016

I've done some tests (only for fma(), mig() and diam(), which were corrected in the last commit), using julia 0.4 and 0.5, and considering the current master (commit bf62bf4) and b1f96ba, using Interval{Float64} and Interval{BigFloat}. The results below are for 10^5 iterations of those functions (in that order).

  • JULIA 0.4
    In master:
    Testing: Float64
    2.459652 seconds (53.00 M allocations: 1007.090 MB, 4.59% gc time)
    0.586216 seconds (13.00 M allocations: 289.917 MB, 3.65% gc time)
    0.104019 seconds (6.00 M allocations: 137.329 MB, 10.64% gc time)
    Testing: BigFloat
    8.345536 seconds (56.00 M allocations: 1.669 GB, 17.86% gc time)
    2.010344 seconds (18.00 M allocations: 564.575 MB, 22.14% gc time)
    0.483570 seconds (6.00 M allocations: 175.476 MB, 24.32% gc time)
    For b1f96ba:
    Testing: Float64
    0.033382 seconds (149 allocations: 10.167 KB)
    0.020198 seconds (5 allocations: 176 bytes)
    0.014262 seconds (5 allocations: 176 bytes)
    Testing: BigFloat
    5.557482 seconds (48.00 M allocations: 1.460 GB, 24.38% gc time)
    1.179876 seconds (13.00 M allocations: 411.987 MB, 33.96% gc time)
    0.399777 seconds (4.00 M allocations: 114.441 MB, 28.36% gc time)
  • JULIA 0.5
    In master:
    Testing: Float64
    0.198245 seconds (2.00 M allocations: 30.528 MB, 19.88% gc time)
    0.081418 seconds (1.00 M allocations: 30.518 MB, 3.26% gc time)
    0.062971 seconds (1.00 M allocations: 15.259 MB, 3.84% gc time)
    Testing: BigFloat
    5.417516 seconds (36.00 M allocations: 1.311 GB, 23.16% gc time)
    1.070972 seconds (11.00 M allocations: 396.729 MB, 34.54% gc time)
    0.431670 seconds (4.00 M allocations: 114.441 MB, 24.42% gc time)
    For b1f96ba:
    Testing: Float64
    0.040103 seconds (160 allocations: 11.152 KB)
    0.019922 seconds (5 allocations: 176 bytes)
    0.014244 seconds (5 allocations: 176 bytes)
    Testing: BigFloat
    5.095795 seconds (34.00 M allocations: 1.252 GB, 23.44% gc time)
    0.963879 seconds (9.00 M allocations: 350.952 MB, 35.31% gc time)
    0.340945 seconds (3.00 M allocations: 99.182 MB, 26.63% gc time)

Comparing Julia 0.4 vs 0.5, 0.5 is better (both in master and in b1f96b).

Comparing master vs b1f96b, I think b1f96b does something really good, both in elapsed time as in the allocated memory, and for both Interval cases considered, in particular for Float64s.

@lbenet
Copy link
Member Author

lbenet commented Mar 15, 2016

So I agree with your comment above, that 0.5 solves many things, but b1f96ba too.

@dpsanders
Copy link
Member

I think the origin of the problem is in this kind of thing:

julia> x = @interval(1)
[1.0, 1.0]

julia> @code_warntype convert(Interval{BigFloat}, x)
Variables:
  #s15::Type{ValidatedNumerics.Interval{BigFloat}}
  x::ValidatedNumerics.Interval{Float64}

Body:
  begin  # /Users/dpsanders/.julia/v0.4/ValidatedNumerics/src/intervals/conversion_promotion.jl, line 10:
      return (ValidatedNumerics.make_interval)(T,x::ValidatedNumerics.Interval{Float64})::Any
  end::Any

@dpsanders
Copy link
Member

I don't see how to fix it though. However, if in 0.5 this is no longer a problem, then I don't think it's worth worrying too much about?

@lbenet
Copy link
Member Author

lbenet commented Mar 18, 2016

I agree that somewhere in src/rounding.jl there is something to be done (convert eventually calls make_interval). In your example above, it calls @round, which then calls @setrounding. The latter macro essentially passes a function as an argument to a function. That is the reason that i annotated @setrounding with the expected type.

Though I think this is important (see the benchmarks), if you prefer we can leave this for now, and I'll take a look on this. So we can tag a new version as you wanted.

@dpsanders
Copy link
Member

Since the type instability no longer occurs on 0.5 after the functions rewrite, I am going to close this. If you don't agree, feel free to reopen.

@dpsanders dpsanders closed this Apr 3, 2016
@dpsanders dpsanders deleted the type_stab branch February 13, 2017 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants