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

Fixes for algebraic cudd #99

Open
wants to merge 5 commits into
base: algebraic_cudd
Choose a base branch
from

Conversation

CazSaa
Copy link

@CazSaa CazSaa commented Mar 7, 2025

This pull request includes several changes to the dd/cudd_add.pyx file.

Changes to function definitions and error handling:

  • Changed the return type of Cudd_addLeq from DdNode* to int to reflect its actual return type. (The library did not compile for me without this fix)
  • Replaced the implementation of comparison operators (<, <=, >, >=) with NotImplementedError because the implementation does not work with this return type.

Enhancements to file type support:

  • Added support for the 'dot' file type in ADD.dump.

Improvements to the Function class:

  • Added a value property to the Function class to return the value of a leaf ADD node.

Modifications to _to_dot_recurse function:

  • Updated the label for leaf nodes to display their value instead of 'TRUE' or 'FALSE'.
  • Removed taillabel attributes from edges in the _to_dot_recurse because it is redundant with the edge style.

Remove erroneous shortcut in find_or_add:

While i was playing around with this library I found out that for an ADD i constructed, the structure was different from the one I created using PyCUDD. I traced it down to this erroneous shortcut. I'm guessing you tried to mimic the behaviour of one of the shortcuts that is implemented inside CUDD, but i think there was a mistake.

@johnyf
Copy link
Member

johnyf commented Mar 7, 2025

Thank you for finding these issues, I will review the changes.

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