@evgueni-ovtchinnikov can you look at this one?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
@KrisThielemans: So far as I can see from these two lines:
def make_Poisson_loglikelihood(acq_data, model = 'LinearModelForMean'):
...
obj_fun = PoissonLogLikelihoodWithLinearModelForMeanAndProjData()
~~~~~~~~~~~~~~~~~~
model
argument does not specify the acquisition model. My guess is, it specifies particular Poisson LogLikelihood model - cannot really say anything more useful, I am afraid, being out of my depth here.
Seems there are two different meanings of model here. One being the AcquisitionModel
(which is what @francescaleek expected), the other being the "model" used for the likelihood (e.g., LinearModelForMean
). Model seems like a confusing name for the latter, we should maybe change it (e.g., likelihood_type
).
@evgueni-ovtchinnikov perhaps an error should be thrown here?
def make_Poisson_loglikelihood(acq_data, likelihood_type = 'LinearModelForMean'):
if likelihood_type=='LinearModelForMean':
obj_fun = PoissonLogLikelihoodWithLinearModelForMeanAndProjData()
else:
raise error("unknown likelihood type, here's a list of possible entries...")
For @francescaleek 's desired functionality, we would also need:
def make_Poisson_loglikelihood(acq_data, likelihood_type = 'LinearModelForMean', model=None):
if likelihood_type=='LinearModelForMean':
obj_fun = PoissonLogLikelihoodWithLinearModelForMeanAndProjData()
else:
raise error("unknown likelihood type, here's a list of possible entries...")
if model is not None:
obj_fun.set_acquisition_model(model)
Reopened #567.
I thinl @rijobro's suggestion is very good. The intention was that we'd be able to figure out the likelihood type from the model anyway (after all, we only need to know what the domain and range are). So I even think that the likelihood_type
will become irrelevant.
let's use acq_model
as parameter name