sbi
sbi copied to clipboard
Inconsistent state in `LikelihoodEstimator`
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.
@janfb fyi