search

stan-dev / stan

Stan development repository. The master branch contains the current release. The develop branch contains the latest stable development. See the Developer Process Wiki for details.
https://mc-stan.org
BSD 3-Clause "New" or "Revised" License
2.57k stars 368 forks source link

Update hamiltonians to hold its point type #1653

Closed syclik closed 8 years ago

syclik commented 8 years ago

Feature Request

Description

(This is part of @betanalpha's #1361 implementation.)

There might be a few more uses.

Additional Information

  1. This was completely unrelated to #1361, so should be split out. I like the changes and won't affect the interfaces.
  2. This will cut down the diff of @betanalpha's branch for #1361, so it'll be easier to tell what's going on there.
betanalpha commented 8 years ago

What’s the utility of using a typedef instead of templating everything out?

On Oct 29, 2015, at 2:17 PM, Daniel Lee notifications@github.com wrote:

Feature Request

Description

(This is part of @betanalpha's #1361 implementation.)

Update stan/mcmc/hmc/hamiltonians/base_hamiltonian.hpp: Rename template arguments from M to Model and P to Point. Add typedef of Point to PointType. Update all hamiltonians. Update uses of hamiltonians: remove template parameter P, rename H to Hamiltonian, use Hamiltonian::PointType in place of P. stan/mcmc/hmc/integrators/base_leapfrog.hpp stan/mcmc/hmc/integrators/expl_leapfrog.hpp stan/mcmc/hmc/nuts/base_nuts.hpp stan/mcmc/hmc/static/base_static_hmc.hpp stan/mcmc/hmc/static/dense_e_static_hmc.hpp stan/mcmc/hmc/static/diag_e_static_hmc.hpp stan/mcmc/hmc/static/unit_e_static_hmc.hpp There might be a few more uses.

Additional Information

This was completely unrelated to #1361, so should be split out. I like the changes and won't affect the interfaces. This will cut down the diff of @betanalpha's branch for #1361, so it'll be easier to tell what's going on there. — Reply to this email directly or view it on GitHub.

syclik commented 8 years ago

Great question. This was snuck into the 1361 branch. @betanalpha, want to answer that for us?

I think it simplifies things so I'm in favor of it.

betanalpha commented 8 years ago

Great question. This was snuck into the 1361 branch. @betanalpha, want to answer that for us?

Dammit it’s confusing when you split these PR up like this. Putting a typedef in the Hamltonian removes a bunch of duplicated info in the template arguments which should speed up compilation (somewhat) and avoid conflicting template decorations (more important).

Also

stan/mcmc/hmc/nuts/dense_e_nuts.hpp stan/mcmc/hmc/nuts/diag_e_nuts.hpp stan/mcmc/hmc/nuts/unit_e_nuts.hpp

will have to be updated.