LukasZahradnik / PyNeuraLogic

PyNeuraLogic lets you use Python to create Differentiable Logic Programs
https://pyneuralogic.readthedocs.io/
MIT License
281 stars 18 forks source link

[🐛 Bug Report]: Drawing a template raises NullPointerException #56

Closed martin-krutsky closed 6 months ago

martin-krutsky commented 6 months ago

Describe the bug

On Windows, when using the Template.draw method, I get the following error even though I have GraphViz installed and added to the Path (I tried out both System and User path variables):

Exception                                 Traceback (most recent call last)
File Drawer.java:80, in cz.cvut.fel.ida.drawing.Drawer.drawIntoFile()

Exception: Java Exception

The above exception was the direct cause of the following exception:

java.lang.NullPointerException            Traceback (most recent call last)
File ...\neuralogic\utils\visualize\__init__.py:75, in draw(drawer, obj, filename, show, img_type, *args, **kwargs)
     74 try:
---> 75     drawer.drawIntoFile(obj, os.path.abspath(filename))
     76 except jpype.java.lang.NullPointerException as e:

java.lang.NullPointerException: java.lang.NullPointerException: Cannot invoke "cz.cvut.fel.ida.algebra.weights.Weight.toString(java.text.NumberFormat)" because the return value of "cz.cvut.fel.ida.logic.constructs.template.components.WeightedRule.getWeight()" is null

...

Exception                                 Traceback (most recent call last)
File ...\neuralogic\core\template.py:159, in Template.draw(self, filename, show, img_type, value_detail, graphviz_path, model, *args, **kwargs)
    157 if model is None:
    158     model = self.build(Settings())
--> 159 return draw_model(model, filename, show, img_type, value_detail, graphviz_path, *args, **kwargs)

File ...\neuralogic\utils\visualize\__init__.py:153, in draw_model(model, filename, show, img_type, value_detail, graphviz_path, *args, **kwargs)
    150 template = model.template
    151 template_drawer = get_template_drawer(get_drawing_settings(img_type, value_detail, graphviz_path))
--> 153 return draw(template_drawer, template, filename, show, img_type, *args, **kwargs)

File ...\neuralogic\utils\visualize\__init__.py:77, in draw(drawer, obj, filename, show, img_type, *args, **kwargs)
     75         drawer.drawIntoFile(obj, os.path.abspath(filename))
     76     except jpype.java.lang.NullPointerException as e:
---> 77         raise Exception(
     78             "Drawing raised NullPointerException. Try to install GraphViz (https://graphviz.org/download/) on "
     79             "your Path or specify the path via the `graphviz_path` parameter"
     80         ) from e
     82     return
     84 data = drawer.drawIntoBytes(obj)

Exception: Drawing raised NullPointerException. Try to install GraphViz (https://graphviz.org/download/) on your Path or specify the path via the `graphviz_path` parameter

The exception seems to be at least misleading, as can be seen from the two examples below.

Steps to reproduce the behavior

While the following small example works just fine:

from neuralogic.core import Template, R, V

feature_dim = 1
hidden_dim = 3
output_dim = 1

template = Template()

template += (R.layer_1(V.X)[hidden_dim, feature_dim] <= (R.feature(V.Y), R._edge(V.Y, V.X)))
template += (R.predict[output_dim, hidden_dim] <= R.layer_1(V.X))
# {3, 1} layer_1(X) :- feature(Y), *edge(Y, X).
# {1, 3} predict :- layer_1(X).

template.draw('fig.png')

possibly suggesting the problem is not with Path or Graphviz, the next example throws the error above:

from neuralogic.core import Template
from neuralogic.nn.module import GCNConv

feature_dim = 1
hidden_dim = 3
output_dim = 1

template = Template()

template.add_module(
    GCNConv(in_channels=feature_dim, out_channels=hidden_dim, output_name="h0", feature_name="node", edge_name="edge")
)
# h0__edge(I, I).
# h0__edge(I, J) :- edge(I, J). [transformation=identity]
# h0__edge/2 [transformation=identity]
# h0__edge_count(I, J) :- h0__edge(J, X). [transformation=identity, aggregation=count]
# h0__edge_count(I, J) :- h0__edge(I, X). [transformation=identity, aggregation=count]
# h0__edge_count/2 [transformation=inverse, combination=product]
# {3, 1} h0(I) :- node(J), h0__edge(J, I), sqrt(h0__edge_count(J, I)). [transformation=identity, combination=product, aggregation=sum]
# h0/1 [transformation=identity]
template.draw('fig.png')  # <-- throws an error

Expected behavior

Drawing the graph representation of the template.

Environment

Windows 11 Home 10.0.22631 Python 3.12.3 neuralogic 0.7.11 torch 2.2.2 torch-geometric 2.5.2

Additional context

No response

LukasZahradnik commented 6 months ago

Hi, thanks for reporting the bug.

It's caused by adding self-loops, which do not have assigned values:

h0__edge(I, I).
h0__edge(I, J) :- edge(I, J). [transformation=identity]
h0__edge/2 [transformation=identity]
h0__edge_count(I, J) :- h0__edge(J, X). [transformation=identity, aggregation=count]
h0__edge_count(I, J) :- h0__edge(I, X). [transformation=identity, aggregation=count]
h0__edge_count/2 [transformation=inverse, combination=product]
{3, 1} h0(I) :- node(J), h0__edge(J, I), sqrt(h0__edge_count(J, I)). [transformation=identity, combination=product, aggregation=sum]
h0/1 [transformation=identity]

@GustikS Do we just put 1.0 as its value? Hopefully, it should be fine with other edges having different shapes.

GustikS commented 6 months ago

Hi, thanks both.

ha, so this is the new (recent) GCN version with normalization (self-loops)... yes, I'd add them with value 1.0 (same as PyG?) Btw. I'm missing the previous unnormalized version then (there should be flags in the constructor?)

In any case, I've updated the backend to draw templates even with null weights... the GCN template draws fine then: https://github.com/GustikS/NeuraLogic/commit/042bf9edec0031f970f12a2f890248d0736d462c

GustikS commented 6 months ago

But I think there might be some other (unrelated) problem with the graphviz, as I was getting some similar error after updating to the new version while just initializing/importing things... but unable to reproduce anymore (perhaps Martin does), but that's a possibly different issue, so this one can be closed I guess..?

martin-krutsky commented 6 months ago

I can't really speak about the initialization problems, but then I would guess it is more about Graphviz than neuralogic, see, e.g., this discussion. Feel free to close this issue, if it seems to be working now.

LukasZahradnik commented 6 months ago

@martin-krutsky Should be fixed in version 0.7.13. Can you confirm it works fine?

martin-krutsky commented 6 months ago

Yes, drawing the GCNConv template works now. Thanks!