Skip to content

[NFC] base_hmc const accessor to _z #2946

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.

Sign up for GitHub

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

Merged
bob-carpenter merged 1 commit into from
Aug 5, 2020
+30 −0
Merged

[NFC] base_hmc const accessor to _z #2946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/stan/mcmc/hmc/base_hmc.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@
namespace stan {
namespace mcmc {

/**
* Base class for Hamiltonian samplers.
*
* @tparam Model The type of the Stan model.
* @tparam Hamiltonian The type of Hamiltonians over the (unconstrained)
* parameter space.
* @tparam Integrator The type of integrator (e.g. leapfrog).
* @tparam BaseRNG The type of random number generator.
*/
template <class Model, template <class, class> class Hamiltonian,
template <class> class Integrator, class BaseRNG>
class base_hmc : public base_mcmc {
Expand Down Expand Up @@ -132,8 +141,22 @@ class base_hmc : public base_mcmc {
this->z_.ps_point::operator=(z_init);
}

/**
* Gets the current point in the (unconstrained) parameter space.
*
* @return The current point in the (unconstrained) parameter space.
*/
typename Hamiltonian<Model, BaseRNG>::PointType& z() { return z_; }

/**
* Gets the current point in the (unconstrained) parameters space.
*
* @return The current point in the (unconstrained) parameters space.
*/
const typename Hamiltonian<Model, BaseRNG>::PointType& z() const noexcept {

@bob-carpenter bob-carpenter Jul 31, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Needs documentation. This is very simple here. There may not be doc elsewhere, but we're requiring it for new additions and appreciate it getting backfilled where it's missing.

  2. Needs a test. This can be really simple, but we just want to make sure we have tests in place to avoid future regressions.

@kedartal kedartal Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

return z_;
}

virtual void set_nominal_stepsize(double e) {
if (e > 0)
nom_epsilon_ = e;
Expand Down
7 changes: 7 additions & 0 deletions src/test/unit/mcmc/hmc/base_hmc_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ TEST(McmcBaseHMC, point_construction) {
EXPECT_EQ(static_cast<int>(q.size()), sampler.z().g.size());
}

TEST(McmcBaseHMC, point_access_from_const_hmc) {
rng_t base_rng(0);
const stan::mcmc::mock_hmc sampler(stan::mcmc::mock_model(2), base_rng);
EXPECT_EQ(2, sampler.z().q.size());
EXPECT_EQ(2, sampler.z().g.size());
}

TEST(McmcBaseHMC, seed) {
rng_t base_rng(0);

Expand Down