LSSTDESC / firecrown

DESC Cosmology Likelihood Framework
BSD 3-Clause "New" or "Revised" License
29 stars 7 forks source link

Save predictions to sacc #346

Closed tilmantroester closed 6 months ago

tilmantroester commented 10 months ago

A common use case is producing model predictions and then using those prediction as mock data for some downstream tasks. For that the model predictions need to be saved to a sacc object, for which the functionality does not yet exist in firecrown.

There are multiple implementations of this floating around in DESC (e.g. TXPipe, augur, as well as a number of notebooks), standardising one implementation in firecrown would reduce the overall maintenance burden across the various projects that rely on this functionality.

vitenti commented 9 months ago

If we understood correctly your request and your PR, the additional information that would be necessary to use a computed theory vector to update a SACC object would be the list of indices. In this case we can save the SACC indices inside likelihood and have a method to return it. Thus, we can create a separate function that take SACC, theory vector and indices as input and update the SACC object. What do you think about this solution, since updating the SACC object is not part of the likelihood workflow we think that a separated function would be easier to test. We have however a question:

tilmantroester commented 9 months ago

This could be done in a separate function as well. My reason for putting it in Likelihood is that this allows access to the setup of the statistics and their internal state. For example to allow more sophisticated checking of which parts of the data vector are being saved. I don't think assuming a specific workflow is a good approach here. Different users have different workflows. For example, the current design around a sampling workflow is ill-suited for a modelling workflow, which is already causing problems.

As for the issue with the caching: firecrown already caches the predictions (at least for two-point stats) and uses them downstream, get_theory_vector just gives a proper API for this. This is useful whenever you want to do anything beyond computing the chi2, such as plotting or saving the predictions. firecrown has a very stateful design, so this doesn't seem out of place to me.

tilmantroester commented 6 months ago

This has been addressed with #390 and #349.