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

Profile Sampler #342

Open
wants to merge 61 commits into
base: master
Choose a base branch
from
Open

Conversation

ggalloni
Copy link
Contributor

@ggalloni ggalloni commented Nov 30, 2023

Hello, as anticipated I have been working on implementing a profile likelihood pipeline within Cobaya.
This implementation is a new sampler, based on the current minimizer, but which loops over a list of requested values of a parameter. Indeed, I used minimize as a template to write profile. Mimicking minimize, the ignore_prior flag will control whether the profiled quantity is the likelihood or the posterior.

The main modifications are:

  • Of course, some new input quantities were needed to define which parameter to profile and its values. I did the latter in two alternative ways. The default one would be to pass a list of values; the second consists of passing the first and the last values and how many steps to go from one to the other. Note that I designed this in such a way that the profiled parameter must be also part of the params block. The idea is the same as minimize: the most convenient thing to do is to first run a Monte Carlo and afterward do a profiling. This will ensure that the starting point of each profiled point is close enough to the result (this is because even if the profiled parameter is taken different from the absolute best fit, the others will not move too much).
  • I needed to define more initial points, so I looped that part of initialize to generate them and store them.
  • The same thing is done in run which now will minimize all the sampled parameters for each requested point of the profiled one.
  • I modified the way results are processed to store each minimum and create a single text file in the end storing all the relevant information. This file contains essentially a small SampleCollection with the values of all parameters plus the profiled one. This allows the user to also have in the end the so-called co-profiles for each parameter (minimized parameter as a function of the profiled one), which for example gives interesting information on degeneracies. The name of this file will vary depending on ignore_prior: the prefix is either like_profile of post_profile.
  • I added a test file to verify the profile from the same Gaussian distribution used in the test_minimize.py and a test the new output file.
  • I added some documentation on profile using again minimize as a template. Also, I added this sampler to the index of the docs.

Travis passed all tests in my fork, but I was wondering whether I should add an example in the quickstart docs. What do you think?
Let me know if the implementation is satisfactory to you or if I should make any modifications.

@cmbant
Copy link
Collaborator

cmbant commented Jan 3, 2024

Thanks for this (and sorry for delay in looking at it). There does seem to be quite a bit of duplicated code, could you perhaps inherit Profile from Minimize (or both from a common ancestor), so that common code can be reused? (e.g. _get_desc seems basically copied, and other code e.g. setting up minimizers could also probably be refactored into an inherited method?).

@ggalloni
Copy link
Contributor Author

ggalloni commented Jan 3, 2024

Hello @cmbant and happy New Year :)
Yeah, I agree with that. I wasn't sure it was a good idea to design a sampler that inherits from another one, but, since you suggest doing so, I will refactor this to include the least new code possible.
I guess this will make it easier to maintain too.

I'll come back to you when I have relevant news (it may take some time since I am also writing my manuscript ): )

@ggalloni
Copy link
Contributor Author

Hello @cmbant, I tried to inherit from Minimizer to define the Profiler and comment out all the code that was duplicated.

Still, I cannot see the log from Travis to understand what is breaking.
I don't know if it is a problem due to me not having a subscription.

Do you have any insight?

@cmbant
Copy link
Collaborator

cmbant commented Feb 27, 2024 via email

@ggalloni
Copy link
Contributor Author

Yeah, I see the job, but the log only gives this message:

We're sorry, but this data is not available. Please check the repository settings in Travis CI.

@cmbant
Copy link
Collaborator

cmbant commented Feb 27, 2024

Try in a new incongito window, that works fine for me:
https://app.travis-ci.com/github/CobayaSampler/cobaya/jobs/617467186

@ggalloni
Copy link
Contributor Author

That worked!
I'll proceed with the implementation then, thanks!

I rely on multiple initialization of Minimize to fetch the initial points and define all the relevant attributes. Then, I drop the profiled parameter from each quantity to recover the profiled case.
@ggalloni
Copy link
Contributor Author

Ok, now it works.

I inherited from Minimize as you suggested and try to recycle as much code as possible. If I didn't miss some possible shortcuts, I should have kept only the necessary lines.

To initialize the points I rely on multiple initializations of Minimize. Then, I drop the profiled parameter from the relevant quantities. This saves quite a lot of lines in the profile sampler.
Then, I think I cannot avoid to "overwrite" the run method. I tried to recycle the Minimize one but it must be model-specific (I define different models for each value of the profiled parameter) and I cannot modify the model Minimize uses (as far as I know).
Also, the processing of the results and the relative products are modified wrt Minimize.

Let me know if you think I could improve it in some way. Still, I'll keep improving it if I can.

@cmbant
Copy link
Collaborator

cmbant commented Mar 6, 2024

Thanks, looks good. I don't think you need to inherit from CovmatSampler again as Minimize already does.

Jesus suggests you consider making this an external package (which means you'd more obviously get credit for it! And reduce code bloat/maintainance in cobaya itself). You can use external samplers (as for external likelihood packages), though in this case we may need to make some small changes to run.py etc which Jesus said he could help with.

@JesusTorrado
Copy link
Contributor

JesusTorrado commented Sep 9, 2024

Hi @ggalloni

I am very sorry for the delay on this.

Could you please create a minimal package with just the sampler code following the example repo at https://github.com/CobayaSampler/example_sampler/? And then please try to import it in the input as [package.Class] as in the example code? I just want to make sure that it can be imported that way. It won't fully work, of course, because we need to merge the changes in this PR for the general Cobaya files, but we will do that this week. But first let's check that the packaging works.

@cmbant the example external sampler works the same as the likelihoods. If it's a package that has been created explicitly for Cobaya, it's a bit of a shame that we need to import as [package.Class]. Maybe if we agree on a general class name, such as CobayaWrapper for the Sampler-inherited class, we can invoke just using the package name? Not very important, just to have your opinion.

@cmbant
Copy link
Collaborator

cmbant commented Sep 10, 2024 via email

@JesusTorrado
Copy link
Contributor

@cmbant Thanks! Updated. We have some many ways to interface that I often do not remember all of them.

@ggalloni
Copy link
Contributor Author

Hi @JesusTorrado,

here you can find the module with the profile sampler.

I tested it both with tests and by running locally cobaya-run run_profile_example.yaml, and it works fine.

Note that I had to specify a python_path in the example yaml to make it work properly with cobaya-run.
I guess this also applies to your example_sampler as I couldn't run it without doing the same.
Am I doing something wrong?

Let me know if you need me to do something else.

@JesusTorrado
Copy link
Contributor

Hi @ggalloni

Sorry, I forgot to add that it works without the python_path only if it has been installed with pip. I just checked that it does work for my example_sampler and your profiler when they are pip-installed (otherwise, python_path is needed). So nothing to do there (but you may want to clarify it in your docs).

Let me see about the rest of the changes in the PR. I will propose another one soon handling the documentation part, but let's keep this open until then.

But in any case, your profiler works now as an external package, so I would personally drop the reference to the PR in the README and make it standalone.

@ggalloni
Copy link
Contributor Author

Great, thanks!

I'll work on the README asap 👍

@JesusTorrado
Copy link
Contributor

No worries, and apologies again for the huge delay!

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

Successfully merging this pull request may close these issues.

4 participants