# Best way to make myisapprox(a,b;kwds...) = Base.isapprox(a,b;kwds...) type stable?

**URL:** <https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921>\
**Category:** General Usage\
**Created:** [May 25, 2017, 11:36am UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921 "2017-05-25T11:36:04Z")\
**Posts on this page:** 7\
**Page:** 1

<div class="post-metadata">

**Author:** ![dlfivefifty](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/dlfivefifty/32/1959_2.png) [@dlfivefifty](https://discourse.julialang.org/u/dlfivefifty)\
**Post date:** [May 25, 2017, 11:36am UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/1 "2017-05-25T11:36:04Z")

</div>

It is sometimes useful to extend Base functions behaviour by creating another function that passes through, for example:

```julia
myisapprox(a,b;kwds...) = Base.isapprox(a,b;kwds...)
# other overrides of myisapprox, which may include Base types
# so you don't want to overload Base.isapprox directly

```

Unfortunately, the above override is not type stable in 0.6-rc1:

```julia
julia> @code_warntype myisapprox(1.0,2.0)
Variables:
  #self#::#myisapprox
  a::Float64
  b::Float64

Body:
  begin 
      return $(Expr(:invoke, MethodInstance for #myisapprox#1(::Array{Any,1}, ::Function, ::Float64, ::Float64), :(Main.#myisapprox#1), :($(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Any,1}, svec(Any, Int64), Array{Any,1}, 0, 0, 0))), :(#self#), :(a), :(b)))
  end::Any

```

Is there a way to say “copy all definitions of `Base.isapprox` to `myisapprox`, so type stability is maintained”? I suppose I could copy all of the keywords from Base, but this is annoying, as if I just overloaded `Base.isapprox` directly I wouldn’t need to do this.

---

<div class="post-metadata">

**Author:** ![giordano](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/giordano/32/2166_2.png) [@giordano](https://discourse.julialang.org/u/giordano)\
**Post date:** [May 25, 2017, 12:45pm UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/2 "2017-05-25T12:45:10Z")

</div>

For the specific case of `isapprox` that always returns a `Bool`:

```julia
myisapprox(a,b;kwds...)::Bool = Base.isapprox(a,b;kwds...)

```

Note that `isapprox` itself is unstable with keywords:  
[https://github.com/JuliaLang/julia/issues/21794](https://github.com/JuliaLang/julia/issues/21794)

Interestingly, the type-stable version looks slower than the unstable one:

```julia
julia> myisapprox(a,b;kwds...) = Base.isapprox(a,b;kwds...)
myisapprox (generic function with 1 method)

julia> @benchmark myisapprox(1.0, 2.0)
BenchmarkTools.Trial: 
  memory estimate: 80 bytes
  allocs estimate: 1
  --------------
  minimum time: 31.179 ns (0.00% GC)
  median time: 34.251 ns (0.00% GC)
  mean time: 39.566 ns (10.66% GC)
  maximum time: 1.622 μs (97.12% GC)
  --------------
  samples: 10000
  evals/sample: 994

julia> myisapprox(a,b;kwds...)::Bool = Base.isapprox(a,b;kwds...)
myisapprox (generic function with 1 method)

julia> @benchmark myisapprox(1.0, 2.0)
BenchmarkTools.Trial: 
  memory estimate: 80 bytes
  allocs estimate: 1
  --------------
  minimum time: 93.442 ns (0.00% GC)
  median time: 96.119 ns (0.00% GC)
  mean time: 102.693 ns (4.08% GC)
  maximum time: 1.430 μs (91.55% GC)
  --------------
  samples: 10000
  evals/sample: 951

```

---

<div class="post-metadata">

**Author:** ![ChrisRackauckas](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/chrisrackauckas/32/77_2.png) [@ChrisRackauckas](https://discourse.julialang.org/u/ChrisRackauckas)\
**Post date:** [May 25, 2017, 2:52pm UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/3 "2017-05-25T14:52:37Z")

</div>

> [@giordano](#):
>
> Interestingly, the type-stable version looks slower than the unstable one:

That’s standard. Since it’s likely inlining the function call, your version is the exact same thing, except with an extra type-check / conversion afterwards. So your version is doing strictly more work.

Type-instability (or actually here, type-inference) issues are not necessarily a performance issue at their source. The issue is that, if you use the result of a type that is not strictly inferred, then code further down will not know the type, causing dynamic dispatch in function calls and “propagation of the type-uncertainties” (i.e. causing everything else downwards to not be strictly inferred if you’re not careful!). So lack of type inference itself isn’t an issue, rather it’s the fact that lack of inference in one place can make it impossible to infer the types in further code.

I think the problem may be due to keyword arguments. I don’t think there’s a better fix until keyword arguments actually dispatch correctly (there’s a PR for that), so a manual conversion should be fine. It’s ~60ns, you’ll live.

---

<div class="post-metadata">

**Author:** ![dlfivefifty](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/dlfivefifty/32/1959_2.png) [@dlfivefifty](https://discourse.julialang.org/u/dlfivefifty)\
**Post date:** [May 25, 2017, 10:25pm UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/4 "2017-05-25T22:25:32Z")

</div>

I’m more concerned with allocations than time, as my particular use case builds up to MBs. That’s very interesting that using keywords causes such a big difference:

```julia
julia> @btime isapprox(1.0,2.0;rtol=0.0001)
  249.653 ns (3 allocations: 128 bytes)
false

julia> @btime isapprox(1.0,2.0)
  6.796 ns (0 allocations: 0 bytes)
false

```

---

<div class="post-metadata">

**Author:** ![dlfivefifty](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/dlfivefifty/32/1959_2.png) [@dlfivefifty](https://discourse.julialang.org/u/dlfivefifty)\
**Post date:** [May 25, 2017, 10:42pm UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/5 "2017-05-25T22:42:31Z")

</div>

Hmm, just realized `myeps(x...) = Base.eps(x...)` also destroys type stability for `myeps(Float64)... I would have thought this would be type stable, as the `x…`would infer the type of`x`as a`Tuple{Float64}`.

---

<div class="post-metadata">

**Author:** ![ChrisRackauckas](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/chrisrackauckas/32/77_2.png) [@ChrisRackauckas](https://discourse.julialang.org/u/ChrisRackauckas)\
**Post date:** [May 26, 2017, 1:13am UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/6 "2017-05-26T01:13:13Z")

</div>

> [@dlfivefifty](#):
>
> That’s very interesting that using keywords causes such a big difference

My take is, it’s simply a pre-1.0 issue and the solution is right around the corner:

> <https://github.com/JuliaLang/julia/pull/16580>
>
> I tried a couple approaches for kwargs but this one I like.
> It introduces a stru…ctural named-fields counterpart to tuple types (unimaginatively called \`Struct\`). You can make a \`Struct(x = 1.0, y = 2.0)\` and it has type \`Struct{(:x,:y),Tuple{Float,Float}}\` with fields \`x\` and \`y\`.
> 
> There is a couple functions to merge or diff them and it uses that as the basis for kwargs splatting and unpacking.
> 
> This PR is not ready yet and is up here mostly to discuss the implications. The problematic one are probably over-specialization and the use of staged functions in the implementation (as always).
> 
> Assuming we want this, here are a few points I'm wondering about :
> \- Should we aim to merge those things with \`Tuple\` so that we have only one kind of structural type. We'd get dispatch-on-kwargs for "free" (== retrofit all code that touches tuple types ... hem). This is probably a long term question and keeping them separate is a decent first step in that direction anyway.
> \- Should they be covariant ? I'm guessing yes.
> \- Should they have vararg forms ? I'm guessing yes too, so you could match on "at least those fields with those types"
> \- Should they have an optional-field form ? Like \`{x::Int, y?::Int}\` that has \`{x::Int,y::Int}\` and \`{x::Int}\` as subtypes. I tink this is necessary if we want to express our current kwarg semantics using only normal dispatch and avoid generating a lot of methods.
> \- Should we always sort fields ? For now \`Struct\` types take ordering into account but the kwarg lowering/merging/diffing always keeps them sorted to avoid generating a specialization for every permutation.
> 
> Other than that, it currently seem to work when used as a replacement for our current kwarg lowering and we get specialization, inlining, inference etc.
> Going forward with this we can probably remove a bit of the \`kwfunc\` business but for now I only did minimal integration.
> 
> I feel that the \`Struct\` type is interesting in itself anyway.

so I wouldn’t let it influence designs. Just design assuming it will infer correctly in a few months.

---

<div class="post-metadata">

**Author:** ![dlfivefifty](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/dlfivefifty/32/1959_2.png) [@dlfivefifty](https://discourse.julialang.org/u/dlfivefifty)\
**Post date:** [May 26, 2017, 3:13am UTC](https://discourse.julialang.org/t/best-way-to-make-myisapprox-a-b-kwds-base-isapprox-a-b-kwds-type-stable/3921/7 "2017-05-26T03:13:24Z")

</div>

A few months away is a bit optimistic for a PR that just 7 days ago was getting looked again. Unless you are one of the people who thinks 1.0 is still going to be released at JuliaCon 2017 😜

But yes, on the software side you are right about just waiting.