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

#1227 - Efficient subset check between CartesianProducts #2241

Merged
merged 2 commits into from
Jul 22, 2020

Conversation

schillic
Copy link
Member

@schillic schillic commented Jul 19, 2020

Closes #1227.

Additionally this PR adds a subset check between CartesianProductArrays of different block structure by using the fallback implementation.

Tests were already present.

# two non-subset blocks

julia> @time X  Y
  0.000024 seconds (111 allocations: 7.453 KiB)
false

julia> @time (X, Y, true)
  0.000032 seconds (151 allocations: 9.891 KiB)
(false,   [1]  =  -0.989371
  [2]  =  0.890032
  [3]  =  1.15984
  [4]  =  2.03698)

### this branch

julia> @time X  Y
  0.000003 seconds
false

julia> @time (X, Y, true)
  0.000005 seconds (4 allocations: 272 bytes)
(false, [-0.9893707748634932, 0.6090025573953656, 0.6279955152649557, 0.38677706906864234])


# first block is a subset

julia> @time X  Y
  0.000025 seconds (129 allocations: 8.672 KiB)
false

julia> @time (X, Y, true)
  0.000030 seconds (163 allocations: 10.703 KiB)
(false,   [1]  =  -0.427312
  [2]  =  0.890032
  [3]  =  1.15984
  [4]  =  2.03698)

### this branch

julia> @time X  Y
  0.000003 seconds
false

julia> @time (X, Y, true)
    0.000005 seconds (6 allocations: 384 bytes)
(false, [-0.23845042779673375, 0.6386840809336349, 1.159841586170895, 0.6386840809336349])


# both blocks are subsets

julia> @time X  Y
  0.000025 seconds (147 allocations: 9.891 KiB)
true

julia> @time (X, Y, true)
  0.000026 seconds (149 allocations: 10.000 KiB)
(true, Float64[])

### this branch

julia> @time X  Y
  0.000003 seconds
true

julia> @time (X, Y, true)
  0.000004 seconds (6 allocations: 336 bytes)
(true, Float64[])

@schillic schillic requested a review from mforets July 19, 2020 14:30
@mforets
Copy link
Member

mforets commented Jul 20, 2020

there is a problem to extend ⊆(::CartesianProduct{N}, ::CartesianProduct{N}, ::Bool=false) .. without dispatch on the algorithm, in the sense that the new method doesn't apply for cases which have different block structure, but which are polytopic, so the fallback implementation (_issubset_constraints_list) applies.

one approach would be to handle the case where the blocks dims don't match instead of throwing an error. otoh, this pr shows that we don't test x ⊆ y if x = rand(Interval) × rand(BallInf) and y = rand(BallInf) × rand(Interval).

@schillic
Copy link
Member Author

True, but the same holds for the CPA method. I copied that behavior as suggested in the issue. I can fix them here.

@mforets
Copy link
Member

mforets commented Jul 20, 2020

sorry, what i meant in my comment is that if we merge this PR there is a regression as some subset checks no longer work as the one i gave.

True, but the same holds for the CPA method.

ok, i see. then we should fix that too :)

@schillic
Copy link
Member Author

ok, i see. then we should fix that too :)

done ⛑️

@mforets
Copy link
Member

mforets commented Jul 22, 2020

cool!

return invoke(⊆, Tuple{LazySet{N}, LazySet{N}, Bool}, X, Y, witness)

i think that using internal functions is better than invoke, but we can fix that later.

julia> @time ⊆(X, Y, true)
0.000005 seconds (6 allocations: 384 bytes)

when measuring small times (like hundreds of ms or smaller), @btime from BenchmarkTools is better because the differences shown can be just variation between runs.

@schillic
Copy link
Member Author

schillic commented Jul 22, 2020

when measuring small times (like hundreds of ms or smaller), @btime from BenchmarkTools is better because the differences shown can be just variation between runs.

Yeah, I mainly looked at the memory usage here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Efficient subset check between CartesianProducts
2 participants