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

Implement likelihood/posterior distinction (in output of estimators and ingestion in summarizers) #2

Open
MarkusMichaelRau opened this issue Mar 20, 2023 · 4 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@MarkusMichaelRau
Copy link

Not all summarizers are compatible with all types of estimators. RAIL implements methods that are conditional density estimates; others evaluate likelihoods. Summarizers that deconvolve individual galaxy redshift likelihoods would not necessarily be compatible with conditional density estimates. Similarly, summarizers that expect conditional density estimates would not give sensible results for likelihoods.

Ensuring that a summarizer can only be used with a correct estimator would be helpful. This could be maybe ensured using a factory pattern for summarizers and estimators.

@aimalz
Copy link
Collaborator

aimalz commented Mar 21, 2023

The fix for this isn't necessarily about the estimator -- BPZ with a nontrivial prior still yields a posterior rather than a likelihood -- so the software fix corresponding to this issue is to associate metadata of "likelihood" (as opposed to "posterior" or None) with the qp.Ensemble output by an estimator when it is run such that likelihoods are produced (and I think the only estimator we have that can do that right now is BPZ when given a flat prior). Then the varInference summarizer would need to check that its inputs contain this metadata.

@aimalz aimalz added the good first issue Good for newcomers label Mar 27, 2023
@aimalz aimalz changed the title Implement estimator specification for summarizers Implement likelihood/posterior distinction (in output of estimators and ingestion in summarizers) Mar 30, 2023
@aimalz
Copy link
Collaborator

aimalz commented May 24, 2023

Note: when this issue is closed, a new issue should be created for wrapping CHIPPR as another summarizer.

@eacharles eacharles transferred this issue from LSSTDESC/rail_attic Jun 13, 2023
@aimalz
Copy link
Collaborator

aimalz commented Aug 4, 2023

As per discussion with @sschmidt23, this boils down to RAIL estimators associating a keyword to their output ensembles for "likelihood" or "posterior," but it touches on the related requirement for #14 to have creators and estimators associate "reference" or "estimate" to their output ensembles to be checked before calculating certain metrics.

@jlvdb
Copy link

jlvdb commented May 21, 2024

I would like to add to this particular issue and mention that my yet_another_wizz wrapper produces jackknife samples, which have a different variance than bootstrap samples that one might naively expect. For now I will just tag the resulting Ensembles with some metadata that indicate this fact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants