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

New interface for sampling #300

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

vfisikop
Copy link
Contributor

@vfisikop vfisikop commented Apr 15, 2024

Function is parameterized by walk and distribution. Added support for basic uniform all samplers.

@vfisikop vfisikop marked this pull request as draft April 15, 2024 13:37
@vfisikop vfisikop requested a review from TolisChal July 4, 2024 16:51
@vfisikop vfisikop marked this pull request as ready for review July 4, 2024 16:53
@TolisChal TolisChal force-pushed the develop branch 4 times, most recently from 906e119 to f1abc36 Compare July 7, 2024 02:25

auto accelerated_billiard_walk(HPolytopeType& HP, RNGType& rng, unsigned int walk_len, unsigned int num_points)
{
typedef RandomPointGenerator<AcceleratedBilliardWalkType> Generator;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this line here?

Comment on lines +171 to +172
//HP.print();
//std::cout<<"\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we delete them?


auto start = std::chrono::steady_clock::now();

sample_points<64>(HP, q, walk, distr, rng, walk_len, rnum, nburns, samples);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is 64?


Point p = starting_point;

//TODO: burnin for nuts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why burnin is difficult for nuts?

template <typename NT>
static NT compute(HPolytope<Point> const& P)
{
return NT(2) * std::sqrt(NT(P.dimension())) * P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should call ComputeInnerBall() from the Polytope.
We have a flag in Hpolytope that checks if the innerball is already computed in ComputeInnerBall().
If the ball isn't computed then InnerBall() will trigger a runntime error.
I would probably add a todo to do the same check in InnerBall() as well and if the ball is not computed then we call ComputeInnerBall() in InnerBall(). Or we could delete InnerBall() and rename the ComputeInnerBall() to InnerBall().

@@ -346,7 +346,7 @@ struct AcceleratedBilliardWalk
pointset.push_back(_p);
NT rad = NT(0), max_dist, Lmax = get_delta(), radius = P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

{
MT A = P.get_mat();
VT b = P.get_vec(), _vec_point = VT::Zero(P.dimension()), p0 = p.getCoefficients();
NT r = P.ComputeInnerBall().second;
NT r = P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

{
MT A = P.get_mat();
VT b = P.get_vec(), _vec_point = VT::Zero(P.dimension()), p0 = p.getCoefficients();
NT r = params.set_L ? params.m_L
: P.ComputeInnerBall().second;
: P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

{
MT A = P.get_mat();
VT b = P.get_vec(), _vec_point = VT::Zero(P.dimension()), p0 = p.getCoefficients();
NT r = P.ComputeInnerBall().second;
NT r = P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

{
MT A = P.get_mat();
VT b = P.get_vec(), _vec_point = VT::Zero(P.dimension()), p0 = p.getCoefficients();
NT r = params.set_L ? params.m_L
: P.ComputeInnerBall().second;
: P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

{
MT A = P.get_mat();
VT b = P.get_vec(), _vec_point = VT::Zero(P.dimension()), p0 = p.getCoefficients();
NT r = P.ComputeInnerBall().second;
NT r = P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

{
MT A = P.get_mat();
VT b = P.get_vec(), _vec_point = VT::Zero(P.dimension()), p0 = p.getCoefficients();
NT r = params.set_L ? params.m_L
: P.ComputeInnerBall().second;
: P.InnerBall().second;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz call ComputeInnerBall() (see my previous comment)

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

Successfully merging this pull request may close these issues.

2 participants