Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implemented logic circuit and parsing of SDD #312

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

n28div
Copy link
Collaborator

@n28div n28div commented Nov 5, 2024

Parsing of SDD is directly performed from the standard SDD file instead of using pySDD and additional dependencies.
Due to issue #311 the circuit will not work as expected. A quick and dirty patch is to update cirkit/backend/torch/layers/input.py

line 228:

- return self.semiring.map_from(x, LSESumSemiring)
+ return self.semiring.map_from(x, SumProductSemiring)

and line 328:

- logits = torch.log(self.probs()) if self.logits is None else self.logits()
+ logits = self.probs() if self.logits is None else self.logits()

side-effects have not been considered though.

@loreloc
Copy link
Member

loreloc commented Nov 19, 2024

Hi Nicolas,
I have just merged a PR that unifies MixingLayer and DenseLayer in a single class SumLayer. Therefore, could you please try replacing the mixing layers with sum layers in your implementation for this?
Thank you!

@n28div
Copy link
Collaborator Author

n28div commented Nov 22, 2024

Done!

@loreloc
Copy link
Member

loreloc commented Nov 25, 2024

Hi Nicolas,
did you try the code?
This line does not seem correct

sum_node = SumLayer(len(sum_inputs), 1, arity=len(sum_inputs), weight_factory=weight_factory)

The number of input and output units should always be 1 in your case, but arity > 1.

@n28div
Copy link
Collaborator Author

n28div commented Nov 25, 2024

Yep, on my tests it did work just the same as before. You are right though, I misinterpreted the parameter. The new commit should be ok.

@loreloc
Copy link
Member

loreloc commented Nov 27, 2024

Hi Nicolas,
is there something else that needs to be done for this PR?
Otherwise, I can have a look more in detail and merge it by the end of this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants