-
Notifications
You must be signed in to change notification settings - Fork 29
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
ENH: special object for decision rules. #126
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
### | ||
# special struct to hold decision rules | ||
### | ||
|
||
# import Main | ||
struct DRStore{T} | ||
data::Tuple{Vararg{Vector{T}}} | ||
flat::Vector{T} | ||
end | ||
|
||
Main.length(ds::DRStore) = length(ds.data) | ||
Main.size(ds::DRStore, x...) = size(ds.data, x...) | ||
Main.getindex(ds::DRStore,x...) = getindex(ds.data,x...) | ||
Main.setindex!(ds::DRStore,x...) = setindex!(ds.data,x...) | ||
Main.abs(ds::DRStore{T}) where T = abs(ds.flat) | ||
|
||
maxabs(ds::DRStore{T}) where T = maximum(e->norm(e,Inf), ds.flat) | ||
|
||
norm(ds::DRStore{T}) where T = maximum(e->norm(e,Inf), ds.flat) | ||
# Main.norm(ds::DRStore{T},k::Int) where T = norm( (norm(e,k) for e in ds.flat), k) | ||
distance(ds1::DRStore{T}, ds2::DRStore{T}) where T = maximum( k->norm(k[1]-k[2],Inf), zip(ds1.flat,ds2.flat) ) | ||
# distance(ds1::DRStore{T}, ds2::DRStore{T}, k::Int) where T = norm( (norm(e[1]-e[2],k) for e in zip(ds1.flat,ds2.flat)), k ) | ||
|
||
total(ds::DRStore{T}) where T = sum(length(e) for e in ds.data) | ||
|
||
function DRStore(::Type{T}, dims::AbstractVector{Int}) where T | ||
L = sum(dims) | ||
fD = zeros(T, L) | ||
ptr = pointer(fD) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yikes 😨 ! I don't think I'v ever seen Are we confident that this is safe and works correctly? I would guess that it does given your experiments, but it makes me a little scared. What type of penalty would we get if we were to construct There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think problem is not whether it is safe:
In principle, SubArray should work, and we should be able to pass them to all routines (including interpolation routines). Right now it doesn't:
Right now, I don't totally understand how the new reintepret will work in 0.7 and it certainly won't be backward compatible. |
||
sz = sizeof(T) | ||
offsets = [[0]; cumsum(dims)[1:end-1]]*sz | ||
D = [unsafe_wrap(Array{T}, ptr+offsets[i], dims[i]) for i=1:length(dims)] | ||
return DRStore(tuple(D...), fD) | ||
end | ||
function DRStore(data::AbstractVector{S}) where S<:AbstractVector{T} where T | ||
# data is always copied in this case | ||
# as we cannot guarantee data is contiguous | ||
dims = [length(d) for d in data] | ||
ds = DRStore(T, dims) | ||
for i=1:length(data) | ||
ds.data[i][:] = data[i] | ||
end | ||
ds | ||
end | ||
|
||
# this one reuses data in place | ||
# should it be called DRStore! ? | ||
function DRStore(fD::Vector{T},dims::AbstractVector{Int}) where T | ||
L = sum(dims) | ||
ptr = pointer(fD) | ||
sz = sizeof(T) | ||
offsets = [[0]; cumsum(dims)[1:end-1]]*sz | ||
D = [unsafe_wrap(Array{T}, ptr+offsets[i], dims[i]) for i=1:length(dims)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should probably be testing that
(they did warn us it was unsafe) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see anything wrong with the value |
||
return DRStore(tuple(D...), fD) | ||
end | ||
|
||
DRStore! = DRStore | ||
|
||
function Main.copy(ds::DRStore{T}) where T | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I followed the error message without thinking... |
||
dims = [length(e) for e in ds.data] | ||
fD = deepcopy(ds.flat) | ||
ds1 = DRStore!(fD,dims) | ||
return ds1 | ||
end | ||
|
||
Main.copy!(ds0::DRStore{T}, ds1::DRStore{T}) where T = copy!(ds0.flat, ds1.flat) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
|
||
function Main.clamp!(ds::DRStore{T}, ds_lb::DRStore{T}, ds_ub::DRStore{T}) where T | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
N = length(ds.flat) | ||
for n=1:N | ||
ds.flat[n] = clamp.(ds.flat[n], ds_lb.flat[n], ds_ub.flat[n]) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
|
||
import Dolo: DRStore | ||
using StaticArrays | ||
|
||
@testset "DR Storage" begin | ||
|
||
@testset "floats" | ||
|
||
vals = rand(150) | ||
dims = [50,60,40] | ||
ddr = DRStore(vals, dims) | ||
|
||
ddrc = copy(ddr) | ||
|
||
ddr.flat[3] = 19.341 | ||
@test maximum(abs,ddr.flat-vals)==0.0 | ||
@test maximum(abs,ddr.flat-ddrc.flat)!=0.0 | ||
@test maximum(cat(1, ddr.data...) - ddr.flat)==0.0 | ||
|
||
|
||
dd = [rand(4) for i=1:3] | ||
ds = TTest.DRStore(dd) | ||
|
||
@test maximum(cat(1, ds.data...) - ds.flat) == 0.0 | ||
|
||
|
||
ds.flat[end] = 10 | ||
@test maximum(cat(1, ds.data...) - ds.flat) == 0.0 | ||
|
||
ds.data[2][:] *= 0.1 | ||
@test maximum(cat(1, ds.data...) - ds.flat) == 0.0 | ||
|
||
@test TTest.distance(ds,ds) == 0.0 | ||
# @time TTest.distance(ds,ds0) | ||
|
||
end | ||
|
||
begin "sarrays" | ||
|
||
ddr1 = DRStore(SVector{2,Float64}, [20000,30000,40000]) | ||
ddr2 = DRStore(SVector{2,Float64}, [20000,30000,40000]) | ||
|
||
@time maxabs(ddr1) | ||
@time distance(ddr1, ddr2) | ||
|
||
# tbc... | ||
|
||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any kind of penalty from the
[ds0.data...]
that appears throughout? (e.g. performance, memory, or type inference penalty)This might be pushing things too far (and please tell me if it is), but would it make sense to have
drs.data
beNTuple{N,Vector{T}}
so we know how many elements are in it and can use@generated
functions to unroll these[drs.data...]
calls?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I don't think we have any penalty. The elements of
[ds0.data...]
should be correctly inferred.set_values!
itself could be improved to take a tuple as argument. I doubt unrolling would provide much benefit, as the amount of code within the loop is quite big.