facebookresearch/beanmachine

BMG issue with "Internal compiler error in edge requirements checking:"

sdementen opened this issue · 9 comments

sdementen commented

Issue Description

Running the simple model below with BMG raises a

beanmachine.ppl.compiler.internal_error.InternalError: Internal compiler error in edge requirements checking:
 Node of type BetaNode is being checked for requirements but the lattice typer is unable to assign it a type. Requirements checking always needs to know the lattice type of a node when checking requirements on its outgoing edges.

Replacing dist.Bernoulli(1 - self.Probability()) by dist.Bernoulli(self.Probability()) removes the error which looks strange (but it is another model...).

Replacing return dist.Beta(self.MetaParameter() + 0.1, self.MetaParameter() + 0.1) by return dist.Beta(0.1, self.MetaParameter() + 0.1) changes the error to ValueError: Beta parents must be positive real-valued

Steps to Reproduce

import beanmachine.ppl as bm
import torch
import torch.distributions as dist


class Model:
    @bm.random_variable
    def MetaParameter(self):
        return dist.Uniform(0, 1)

    @bm.random_variable
    def Probability(self):
        return dist.Beta(self.MetaParameter() + 0.1, self.MetaParameter() + 0.1)

    @bm.random_variable
    def Observation(self):
        return dist.Bernoulli(1 - self.Probability())


model = Model()

samples = bm.inference.BMGInference().infer(
    queries=[model.MetaParameter()],
    observations={model.Observation(): torch.tensor(1)},
    num_samples=1000,
    num_chains=2,
)

Expected Behavior

I would expect the code to run correctly.
Is there some documentation on how to add new rules to the lattice typer ?

System Info

Please provide information about your setup

  • PyTorch Version (run print(torch.__version__): 1.10.1+cpu
  • Python version: 3.8.12 (default, Oct 12 2021, 03:01:40) [MSC v.1916 64 bit (AMD64)]
ndmlny-qs commented

Thanks for trying out the tool.

I can confirm the errors you are getting with the different parameters you discussed above, as well as the model running when you replace the probability in the Bernoulli. BMGInference is an experimental implementation under active development, and the docstring states that

the number of primitive distributions supported is currently limited.

With that bit of knowledge I replaced the dist.Uniform(0, 1) in your model with dist.Beta(1, 1) and the model ran. @wtaha what are your thoughts about the current errors being shown to the user?

wtaha commented

Thank you very for checking out Bean Machine and BMGInference, and reporting these issues. This is a very interesting use case from the point of view of the internal type checker. I am taking a looking and will hopefully get back later today.

wtaha commented

That's absolutely right, @ndmlny-qs: Replacing dist.Uniform(0,1) with dist.Beta(1, 1) will make the original model work. So, @sdementen, you've found an important identity that we may want our compiler to recognize automatically.

Will continue to further investigate this issue today and will get back to you regarding the lattice typer and how to add rules.

sdementen commented
wtaha commented

You're welcome! With regards to complement, that actually works already once the Uniform is replaced with Beta.

sdementen commented

I confirm it works when replacing Uniform with Beta, tx a lot!

  • ❤️2
ericlippert commented

Thanks for using the compiler and for reporting this defect; apologies for the error. I have a simple fix for it and will land it as soon as possible.

FYI, what's going on here is: we accumulate a Bayesian graph representing the model as it is written in Python, and must then make mutations to that graph that make it a legal input to Bean Machine Graph. BMG has quite strict rules about nodes and edges; we run a series of mutation passes which attempt to get the graph into a legal form or produce an error if we cannot.

BMG assigns a "type" -- probability, real, positive real, and so on -- to each node, and a type requirement on each edge. As we mutate the graph during rewriting, descendant nodes of a mutated node could change type as a result of the mutation.

A typo in the code which propagates the consequences of a mutation throughout the descendant nodes was causing the propagation to end early, which was leaving some nodes untyped in the graph.

The edge requirement checking pass has a precondition that all nodes are correctly typed, and so we got an internal compiler error when that precondition was unexpectedly violated.

Is there some documentation on how to add new rules to the lattice typer ?

End users should not need to add rules to the typer; the lattice typer just enforces rules imposed by BMG.

However, we are considering the possibility of back-end compilation targets other than BMG, and this might necessitate a "pluggable" type system of some sort. (Eric's musings on possible future compiler architectures are for entertainment purposes only.)

There is a separate issue that your model illustrates which is alluded to in a comment above: in this model we do not correctly optimize the 1-beta() operation into a probability-complement operator. I already knew about that issue and will fix it at a later date.

Thanks again!

sdementen commented

Thank you for taking the time to write this detailed comment, very instructive !

  • 👍1
ericlippert commented

This issue should be fixed by pull request #1360; in related news I also have PRs coming up for fixing the 1-beta() optimization error and some improvements to the error messages.