sbi icon indicating copy to clipboard operation
sbi copied to clipboard

Inconsistent state in `LikelihoodEstimator`

Open schroedk opened this issue 8 months ago • 1 comments

Hey, I am new to the sbi package and currently locking at the code as a preparation for the hackathon. While looking at the class LikelihoodEstimator and it's subclasses, it seems to me that it is possible to create an inconsistent state of the object in the following way:

trainer = MNLE()
trainer.append_simulations(...)
trainer.train()
trainer.append_simulations(...)
trainer.build_posterior(...)

the reason is, that trainer also holds the data, but the state of the internal density_estimator is only updated by train. In addition, if one would provide a density_estimator and/or prior to the call of build_posterior, the internal state self._posterior could be completely unrelated to the data and the density_estimator/prior state in the object.

I would like to improve this by thinking about a re-design of these classes with the aim to make a clear separation of concerns and prevent inconsistent state.

The first question would be, does this object need to have state at all.

Edit: the problem regarding the build_posterior method seems to be present in all subclasses of NeuralInterface, which provide an implementation for it.

schroedk avatar Mar 17 '25 07:03 schroedk

@janfb fyi

schroedk avatar Mar 17 '25 07:03 schroedk