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

Possibly bug in step-by-step description #1

Open
pranavashok opened this issue Mar 2, 2020 · 1 comment
Open

Possibly bug in step-by-step description #1

pranavashok opened this issue Mar 2, 2020 · 1 comment

Comments

@pranavashok
Copy link

Hi, I am using the version hosted on the chair webpage and spotted what could possibly be a minor bug. It can be reproduced as follows:

  1. Create formulae x1 & x2 and x1 | x2. This should create an MTBDD with terminals b and c.
  2. Take union of b and c.
  3. Step all the way to the end. The following message is shown

Merging node %1 with c
Done.

I would have expected to see something like "Merging node b U c with c".

@suyjuris
Copy link
Owner

suyjuris commented Mar 2, 2020

Hi! This behaviour is intentional, but pretty bad and I feel that something should be changed. Let me explain:

So, a node has two properties relevant here: name and label. The name is something like “b”, which is short and assigned when the node is finalised (i.e. changed from temporary to final). There is only one name for each node and it does not change. (While it is assigned in the moment the node is finalised, we can use it to render earlier frames, of course, everything is known in advance.)

The label is the thing we currently display on the node. After the node is finalised, this is set to the name of the node, for temporary nodes it usually is something more informative about the current state.

Here, “b U c” is a label. As the node is temporary, it does not have a proper name. Temporary nodes which are never finalised get assigned placeholder names like “%1”. (If you mouse over the node, it shows you its name as well.) Generally, using the label for that would be possible, but it has the problem that sometimes, the label is [3,7,13,16,24,27,31,34,35], i.e. long. Using that does not really work. But using the name runs into the problem you described.

If you have any suggestions, I would very happy to hear them. Right now, I think the best solution is:

  1. Use label instead of name for temporary nodes, but only for boolean formulae / if the name is short.
  2. Add “(labelled ...)” after the name where appropriate.
  3. Maybe change the names of temporary nodes from “%23” to to “temp23”.

(To be clear, I want to do all of these. This is an AND, not OR.) What do you think?

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

No branches or pull requests

2 participants