Skip to content
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

Revise decompose interface #1177

Closed
mforets opened this issue Feb 28, 2019 · 4 comments
Closed

Revise decompose interface #1177

mforets opened this issue Feb 28, 2019 · 4 comments
Assignees
Labels
breaking ❌ This change may break things extension ⬆️ Extension of an existing feature fix 🤕 Fix to a problem that is not too serious refactoring 🔧 Internal code changes, typically no impact on API simplification 👶 Simplifies code

Comments

@mforets
Copy link
Member

mforets commented Feb 28, 2019

If the options provided to decompose are incompatible, it chooses to ignore some and proceed. But this choice is to a certain extent arbitrary.

Proposal: return an error if the options passed to decompose are incompatible, see the Notes for examples.

@mforets mforets added the fix 🤕 Fix to a problem that is not too serious label Feb 28, 2019
@schillic
Copy link
Member

Maybe we should rather use dispatch on the argument type. Having all these options available suggests that you can combine them.

@mforets
Copy link
Member Author

mforets commented Feb 28, 2019

Let's recap on the purpose of decompose:

  1. In short:

Decompose a high-dimensional set into a Cartesian product of overapproximations
of the projections over the specified subspaces.

  1. Currently it accepts 5 options: set_type, ε, blocks, blocks_types, directions. It would be better to simplify the interface and let the checking for inconsistent options be easier. Note that the block https://github.com/JuliaReach/LazySets.jl/blob/master/src/Approximations/decompositions.jl#L229 and https://github.com/JuliaReach/LazySets.jl/blob/master/src/Approximations/decompositions.jl#L237 are almost identical.

  2. Three notions are involved:

  • An input set S (of dimension 2 or higher, since 1 is in some sense trivial)
  • The blocks that define the partition; e.g. two alternatives (*) for a 3-dim S would be [[1], [2, 3]] and [[1], [2], [3]].
  • The set representations which are used for each block, for example in [[1], [2, 3]] one can use an interval for the first dimension and a polygon for dimensions 2 and 3; and for [[1], [2], [3]] one can use intervals in all dimensions.

(*) these are [[1,2,3]], [[1,2],[3]], [[1],[2,3]], [[1],[2],[3]], grouping blocks of variables which are not contiguous is not allowed (?)

  1. Moreover, if one uses e.g. polygon for [2,3] further options can be passed like in epsilon-close approximation; or if template directions are used, one needs to define both the target set e.g. HPolytope and the template directions such as :box or :oct.

Proposal (A) for a combined interface:

    decompose(S::LazySet{N};
              [partition],
              [set_types])::CartesianProductArray where {N<:Real}

Where:

  • partition is a vector of vectors such as [[1], [2, 3]].
  • set_types is a dictionary whose keys are integers (representing the indices of each block) and whose values are of type Union{LazySet, Tuple{LazySet, Pair{Symbol,Any}}}.

Examples (assume that dim(S) = 3):

decompose(S, partition=[[1],[2,3]], set_types=Dict(1=>Interval, 2=>HPolygon))

decompose(S, partition=[[1],[2,3]], set_types=Dict(1=>Interval, 2=>(HPolygon, =>1e-3)))

decompose(S, partition=[[1],[2,3]], set_types=Dict(1=>Interval, 2=>(HPolytope, :oct)))

Proposal (B) for a combined interface. Noting that (A) can be combined even further:

    decompose(S::LazySet{N};
              [partition])::CartesianProductArray where {N<:Real}

Where:

  • partition is a dictionary whose keys are vectors (representing the block) and whose values are of type Union{LazySet, Tuple{LazySet, Pair{Symbol,Any}}}.

Examples (assume that dim(S) = 3):

decompose(S, partition=Dict([1]=>Interval, [2,3]=>HPolygon))

decompose(S, partition=Dict([1]=>Interval, [2,3]=>(HPolygon, =>1e-3)))

decompose(S, partition=Dict([1]=>Interval, [2,3]=>(HPolytope, :oct)))

@mforets
Copy link
Member Author

mforets commented Feb 28, 2019

The proposal doesn't rule out that we overload decompose with other common uses. For example, if i want to decompose a set S using Hyperrectangle and blocks of size 2, one should be able to do decompose(S, block_size=2, set_type=Hyperrectangle) which (after checking that block_size is compatible with dim(S)) makes the appropriate dictionary and calls decompose(::LazySet, ::Dict{Int, Union{LazySet, Tuple{LazySet, Pair{Symbol,Any}}}).

@schillic schillic self-assigned this Feb 28, 2019
@schillic schillic added simplification 👶 Simplifies code refactoring 🔧 Internal code changes, typically no impact on API extension ⬆️ Extension of an existing feature breaking ❌ This change may break things labels Feb 28, 2019
@schillic
Copy link
Member

schillic commented Mar 2, 2019

We decided to go for option (A) above.
The reason against option (B) was that the code does not automatically know at which position in the resulting array it should insert each block result (i.e., for a given block b to identify k such that b is the k-th block). An additional preprocessing would have been necessary here.

schillic added a commit that referenced this issue Mar 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking ❌ This change may break things extension ⬆️ Extension of an existing feature fix 🤕 Fix to a problem that is not too serious refactoring 🔧 Internal code changes, typically no impact on API simplification 👶 Simplifies code
Projects
None yet
Development

No branches or pull requests

2 participants