-
Notifications
You must be signed in to change notification settings - Fork 33
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
WIP #2543 - Hit and run sampling of lazy sets #2548
Conversation
function sample(X::LazySet{N}; kwargs...) where {N} | ||
return sample(X, 1; kwargs...)[1] | ||
end | ||
function sample(X::LazySet{N}, num_samples::Int, sampler::Sampler; |
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.
changing the sampler
from kwarg to positional arg is needed because we have to pass sampler instances -- passing the sampler options outside the struct would be very weird.
in fact, having the set X
wrapped inside the sampler is not required; i would rather rewrite the whole file such that we use multiple dispatch on sample!(D, X::LazySet, ::SamplingAlgorithm; kwargs...)
.
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.
@schillic if you approve the latter idea, or need more clarification, i can address it in a separate PR.
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.
Sorry, I don't understand. We can discuss if you want.
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 example, here the argument is the name of the struct (the type) sampler=LazySets.PolytopeSampler
, but if we want to pass algorithm-specific options, we should be able to pass an instance of the struct.
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.
I see, sure, go ahead 👍
Closing as stale. |
Closes #2543.
Implements the hit and run algorithm. Still WIP (reasons marked with FIXME in the code) yet it works, see notebooks [1] and [2].
[1] https://github.com/mforets/escritoire/blob/master/2021/Week5/HitAndRun.ipynb
[2] https://github.com/mforets/escritoire/blob/master/2021/Week5/HitAndRun_Implementation.ipynb