JakobRobnik/MicroCanonicalHMC

Place sampling subpackage into an mclmc package

Opened this issue · 10 comments

I am trying to integrate your sampler in my code, but when I pip install the package locally, I need to do
import sampling to access the package.

This seems a bit dangerous if you have another local module called sampling.py. It would maybe be better to do:
import mclmc.sampling to follow package naming convention

Thanks for pointing that out! We can change that in the future. Also feel free to change it yourself and submit a PR.

(Should be done soon)

agree! On this note, I'd also suggest migrating to pyproject.toml rather than setup.py in compliance PEP 518 and 621 (more detail here on allowed specifications). And it might be good to standardize the branch structure as well (eg, main vs dev vs numbered branches for issues).

I'm happy to pitch in if there's a need -- the pyproject.toml part would be easy and non-invasive

Sounds good! I added the mclmc/sampling structure (thought haven't exported to pip), but the pyproject.toml is very welcome (currently I'm working on porting the codebase to BlackJax, but feel free to submit a PR).

ok -- I should have a chance to get to this by the end of the week. What branch should I fork from?

I'd recommend forking from master (I should rename to main too), as that is serving effectively as the dev branch. Once it's done, I can re-export to pip.

sorry to ask a potentially silly question, but do you prefer the name mchmc or mclmc for the package? The about and the readme both refer to MCHMC, and the only place mclmc seems to appear is in the directory

made a PR #41 feel free to edit within that as you see fit. I assume the relative paths will be addressed in some other branch you're working on; I just did the minimal amount to pass tests currently

sounds good -- I pushed code and made a PR already so I'll leave it to you or @reubenharry to make edits in the obvious places as necessary. Thanks!