JuliaReach / ReachabilityBenchmarks

JuliaReach benchmark suite
MIT License
4 stars 1 forks source link

Fix includes #1

Closed mforets closed 6 years ago

mforets commented 6 years ago

We shall delete the include line from all models, and LazySets is reexported in Reachability so it can be deleted as well.

Also, the script plot_GEN.jl looks more suitable to go in Reachability.jl/Utils.

schillic commented 6 years ago

I fixed the models. Command for future reference:

find . -type f -regex ".*\.jl" -exec sed -i ':a;N;$!ba;s|include("../../src/Reachability.jl")\n\n||g' {} \;
schillic commented 6 years ago

Also, the script plot_GEN.jl looks more suitable to go in Reachability.jl/Utils.

This script is only meant to plot SpaceEx output and hence is not used in Reachability.jl. As long as we do not want to merge the code for plotting in Reachability.jl with this script, I would keep it here.

mforets commented 6 years ago

Agreed.

With respect to running the benchmarks, i find that julia> include("models/SLICOT/iss/iss.jl"); compute() issued from the folder ReachabilityBenchmarks doesn't work. This is because matopen("iss.mat") seems to only look at the calling folder.

We can simply propose the user to cd to the folder of interest and start julia there as in:

julia> cd("models/SLICOT/iss/")
julia> include("iss.jl")
julia> compute()
mforets commented 6 years ago

if we want to specify relative paths, these 2 ways work:

file = matopen(join(split(@__FILE__, "/")[1:end-1], "/") * "/iss.mat")

and

file = matopen(pwd() * "/models/SLICOT/iss/iss.mat")

but i find the include from the cd from my previous message to be simpler (although these solutions maybe be helpful for the benchmarks suite..)

mforets commented 6 years ago

i take back the last part: the one using the macro @__FILE__ is good because it only requires that the model and its MAT file are in the same place. hence i think we should be using this one.

schillic commented 6 years ago

Yes, I like that. We should probably add this as a function in each model file (not tested):

function load_mat(name::String)::File
    return matopen(join(split(@__FILE__, "/")[1:end-1], "/") * "/" * name * ".mat")
end

The reason is that we want to also include out.mat sometimes. Alternatively we could merge those .mat files into one.

Unfortunately we cannot provide a global function because of @__FILE__. But maybe a function load_mat(path::String, name::String)::File which is called as load_mat(@__FILE__, "iss")?

mforets commented 6 years ago

or we can provide this macro (here or in Reachability/Utils/Utils.jl):

macro relpath(name::String)
    return :(join(split(@__FILE__, "/")[1:end-1], "/") * "/" * $name)
end

which is used as file = matopen(@relpath "iss.mat"), :property => Property(read(matopen(@relpath "out.mat"), "M")[1,:], 7e-4), etc.

schillic commented 6 years ago

Oh yes, using a macro, very good! But why not provide a macro that includes the matopen command as in my function above?

mforets commented 6 years ago

i found matopen(@relpath "iss.mat") to be more clear than creating a new function whose sole purpose is to spell out the macro.

schillic commented 6 years ago

I meant include this in the macro already. But okay, let us go for your macro.

mforets commented 6 years ago

ok! then the current README is fine, and we need to update the models. there is currently no "Assignee", we could start using it.

mforets commented 6 years ago

thx!