-
Notifications
You must be signed in to change notification settings - Fork 19
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
WIP: Noise updates #77
base: main
Are you sure you want to change the base?
Conversation
moving to-do list higher up |
@rossjjennings rebased to y’all’s changes and switched the base branch from |
OK, great! It doesn't look like the history is bungled to me. The tests are still failing, which seems to be because you've added import statements for |
You're importing some other new packages too ( |
I forgot to comment, sorry! This works for me for enterprise-only noise runs with properly updated config files. I am stalled in testing the discovery implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick note.
src/pint_pal/noise_utils.py
Outdated
Recommended to pass model_kwargs and sampler_kwargs from the config file. | ||
Default kwargs given by function `get_model_and_sampler_default_settings`. | ||
Import configuration parameters: | ||
likelihood: choose from ['Enterprise', 'discovery'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to note is to check capitalization for "Enterprise"
~ Joe G.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for pointing this out!
every instance of enterprise
is now lower case.
This goes in the notebook:
|
config file:
for enterprise only:
|
Side note @jeremy-baier we also need to check the hard-coded paths that are inside noise_utils.py and a few other places, as brought up by: #43 ~ Joe G. |
See Joe’s comment. ‘Enterprise’ —> ‘enterprise’ for consistency |
…files; fix typo in setup discovery
hmmm… looks like the tests are still failing. complaining about not being able to load noise chains. i don’t see any test noise chains in the pint pal directory. does anyone know where it’s looking to grab these chains from ? |
@JPGlaser looking at ross’s comment on this issue. i switched things over so that pint_pal is now using laforge to read in and analyze noise chains. for laforge, you just need to point to the directory so hopefully there is less hardcoding there. |
@rossjjennings if you have a chance tomorrow, could you help me figure this one out ? |
@jeremy-baier To quote what I said in Slack: I think what happens with the tests currently is that So a simple thing to do would be to change |
Drafting a PR for 1) chromatic noise model functionality 2) discovery likelihood 3) enterprise extensions Gibbs sampler.
Also taps into
la_forge
functionality for post processing.Chromatic noise models
model_noise
function.Discovery Likelihood
Gibbs Sampler
Requirements
pip install numpyro[cuda] -f https://storage.googleapis.com/jax-releases/jax_cuda_releases.html
(for GPU)pip install -U "jax[cuda12]”
(for GPU)pip install git+https://github.com/jeremy-baier/enterprise_extensions@jgb-chromatic-gibbs
TO DO
Discovery
————
Enterprise Extensions Gibbs
——————————————
Misc
——