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

order of arguments to build_xxx_kernel is arguably confusing (stylistic issue) #22

Open
murphyk opened this issue Oct 1, 2021 · 1 comment

Comments

@murphyk
Copy link
Contributor

murphyk commented Oct 1, 2021

The API for build_xxx_kernel has the form

build_kernel(varargs,  loglikelihood, logprior, data, batch_size, kwargs)

where varargs is a variable number of arguments, depending on the kernel.
For example

 build_sgld_kernel(dt, loglikelihood, logprior, data, batch_size)
 build_sgldCV_kernel(dt, loglikelihood, logprior, data, batch_size, centering_value):
build_sghmc_kernel(dt, L, loglikelihood, logprior, data, batch_size, alpha=0.01, compiled_leapfrog=True)
...

Arguably it might be nicer to put varargs after batch_size, so the first 4 args are always the same, so that if we add methods like stan_warmup, that automatically estimate dt, we don't have to specify it as the first arg. eg.

 build_sgld_kernel(loglikelihood, logprior, data, batch_size, dt)
 build_sgldCV_kernel(loglikelihood, logprior, data, batch_size, dt, centering_value):
build_sghmc_kernel(loglikelihood, logprior, data, batch_size, dt,  L, alpha=0.01, compiled_leapfrog=True)
build_nuts_kernel(loglikelihood, logprior, data, batch_size, num_warmup)
...
@jeremiecoullon
Copy link
Owner

This is a good idea! I'll fix this soon.

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

No branches or pull requests

2 participants