Type stability #21
Type stability #21
Conversation
| E | ||
| struct Segment{TB<:Number,TI,TE} | ||
| a::TB | ||
| b::TB |
There was a problem hiding this comment.
Note that this is a different constraint: currently we're saying that the types of a and b are both subtypes of Number, but here you're saying they have to be the same subtype of Number. Is this intentional?
There was a problem hiding this comment.
yes indeed, I considered the boundaries of the domain would be of the same type, so this was intentional. Would there be some use cases where this is not the case?
There was a problem hiding this comment.
No, not necessarily, I just wanted to make sure the change was intentional.
| end | ||
| (s0,si) = inf1 ? (s2,s1) : (s1,s2) | ||
| if si < 0 # x = s0 - t/(1-t) | ||
| return do_quadgk_common(t -> begin den = 1 / (1 - t); |
There was a problem hiding this comment.
You can use a do block rather than t -> begin.
There was a problem hiding this comment.
it was written like this, I found it a bit dense, I'll rewrite this part
src/QuadGK.jl
Outdated
| for i in 1:length(s) - 1 | ||
| heappush!(segs, evalrule(f, s[i],s[i+1], x,w,gw, nrm), Reverse) | ||
| end | ||
| segs = sort([evalrule(f, s[i], s[i+1], x, w, gw, nrm) for i in 1:length(s)-1], order = Reverse) |
There was a problem hiding this comment.
shouldn't this be heapify! and not sort?
|
If the integrand is not type-stable, at some point you may need to reallocate the |
|
@stevengj a solution would be to make the integrand type stable then? Otherwise the instability will keep propagating through the call stack |
src/QuadGK.jl
Outdated
| for i in 1:length(s) - 1 | ||
| heappush!(segs, evalrule(f, s[i],s[i+1], x,w,gw, nrm), Reverse) | ||
| end | ||
| segs = heapify([evalrule(f, s[i], s[i+1], x, w, gw, nrm) for i in 1:length(s)-1], Reverse) |
| mid = (seg.a + seg.b) * 0.5 | ||
| seg1 = evalrule(f, seg.a, mid, x,w,gw, nrm) | ||
| seg2 = evalrule(f, mid, seg.b, x,w,gw, nrm) | ||
| heappush!(segs, seg1, Reverse) |
There was a problem hiding this comment.
Seems to need some kind of check here for seg1 isa eltype(segs) (similarly for seg2), and reallocate segs if not (will also require a refactored do_quadgk_common routine that can take an existing segs heap as a starting point).
There was a problem hiding this comment.
this would mean all segments don't have the same type? Wouldn't that be strange, boundaries all have the same type?
No way to do enforce this other than to throw an error, and it would be better to keep the routine generic to an arbitrary user-defined integrand. Type-stability should be an optimization, not a requirement.
No, basically the solution is similar to how |
A first step towards fixing #15 #19 #20
Still type unstable:
seemed ok.
To fix it, the API should be changed to take into account the different types