#397 - constrained_dimensions

[kpotomkin]

Closes #397.

[schillic]

Please see my comment here. We want to have a map from location to variables.

Also, please use Int instead of Int64 😉

[mforets]

Also, please use Int instead of Int64

Int is platform-independent.

[mforets]

on the function's name: get_necessary_vars -> nonzero_vars?

[schillic]

on the function's name: get_necessary_vars -> nonzero_vars?

I would rather say constrained_dims or constrained_dimensions, even if for now the function does not work for general sets.
Actually I would say that we should have such a function in LazySets because not every set supports constraints_list and for some sets this functionality can be achieved cheaper than computing the constraints first.

[mforets]

👍 for outsourcing constrained_dimension to LazySets. This proposes to change

...
       for constraint in constraints_list(stateset(HS, mode))
            append!(vars, findall(!iszero, constraint.a))
       end
...

to

     append!(vars, constrained_dims(stateset(HS, mode))

[schillic]

I start losing the overview 😟
JuliaReach/LazySets.jl#766
JuliaReach/LazySets.jl#792

[mforets]

if constrained_dims(::LazySet) (or another speific set type is defined in LazySets), it is still releveant to add (outside LazySets) a methods constrained_dims(::HybridSystem) or with a similar name.

[schillic]

it is still releveant to add (outside LazySets) a methods constrained_dims(::HybridSystem) or with a similar name

Why? We will not use it (we need this information per location).

[mforets]

maybe, but i'm not sure: if this function is used to reason about the options to be passed to an algorithm (eg. to decide which blocks of variables are selected), one can call it over the given HybridSystem as a preprocessing step.

[schillic]

We now support constrained_dimensions(::HPolyhedron), so you can refactor that part.

[mforets]

• i'm still not conviced about the name get_necessary_vars, what do you think about constrained_dimensions(::HybridSystem)?

• we should add an example to test this function. we can add it to solve_hybrid_bouncing_ball.jl and solve_hybrid_thermostat.jl

[schillic]

• we should add an example to test this function. we can add it to solve_hybrid_bouncing_ball.jl and solve_hybrid_thermostat.jl

I thought to have this function private, so we can call it only inside solve. In addition, it will be very 'artificial' test, which will not show real value of this function.

Private functions should also be tested. I do not see why the test would be artificial. You can very well predict and describe the outcome of this function.
I think what @mforets meant with the examples was that you can use the automata created in there. (however, they are boring). In #404 we would then separate these tests again.

[mforets]

I think what @mforets meant with the examples was that you can use the automata created in there. (however, they are boring).

Yes! While we don't have #404, we can just add a call to constrained_dimensions(::HS) in each model file, then @test that the function behaves as expected.

[mforets]

Bump?

I think that it just remains to test this file with bouncing ball and thermostat.