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

Place sampling subpackage into an mclmc package #39

Open
martin-millon opened this issue Nov 10, 2023 · 10 comments
Open

Place sampling subpackage into an mclmc package #39

martin-millon opened this issue Nov 10, 2023 · 10 comments

Comments

@martin-millon
Copy link

martin-millon commented Nov 10, 2023

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

@reubenharry
Copy link
Collaborator

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

@reubenharry reubenharry mentioned this issue Nov 10, 2023
@reubenharry
Copy link
Collaborator

(Should be done soon)

@samueldmcdermott
Copy link
Contributor

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

@reubenharry
Copy link
Collaborator

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).

@samueldmcdermott
Copy link
Contributor

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

@reubenharry
Copy link
Collaborator

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.

@samueldmcdermott
Copy link
Contributor

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

@samueldmcdermott
Copy link
Contributor

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

@JakobRobnik
Copy link
Owner

JakobRobnik commented Nov 17, 2023 via email

@samueldmcdermott
Copy link
Contributor

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!

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

4 participants