Skip to content

Reduce MDD#949

Merged
tias merged 22 commits into
masterfrom
mdd_reduce
May 29, 2026
Merged

Reduce MDD#949
tias merged 22 commits into
masterfrom
mdd_reduce

Conversation

@WoutPiessens
Copy link
Copy Markdown
Collaborator

Continuing on #924 and #945, I added an auxiliary function _reduce() to the MDD global constraint that identifies equivalent nodes: in other words, nodes with the same set of completing suffixes.

@WoutPiessens WoutPiessens marked this pull request as ready for review May 28, 2026 13:39
@WoutPiessens WoutPiessens requested review from IgnaceBleukx and tias and removed request for IgnaceBleukx May 28, 2026 13:39
Copy link
Copy Markdown
Collaborator

@IgnaceBleukx IgnaceBleukx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, some cleanups and improvements for readibility possible.

Please also add a small tests tests/test_globalconstraints.py to ensure the reduce step is working correctly.

Comment thread cpmpy/expressions/globalconstraints.py Outdated
Comment thread cpmpy/expressions/globalconstraints.py Outdated
Comment thread cpmpy/expressions/globalconstraints.py
Copy link
Copy Markdown
Collaborator

@tias tias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from what I see and realize now, the constructor does too much work...

it should just store (arr, transitions), then self.root_node,
and then it can compute self.sink_node by taking set(id2) - set(id1)?

and then the _get_complete_mdd() would become get_mapping(complete=True) which would create those (mapping, levels) for decompose?

so then the question is reduce, the goal is to rewrite/remove rows from the table, right? can that be done more directly?

If not, then at the least I think you should make sure that reduce keeps self.levels and self.mapping correct too?

Comment thread cpmpy/expressions/globalconstraints.py
@tias
Copy link
Copy Markdown
Collaborator

tias commented May 28, 2026

I don't want to complicate things... but I don't like double information either.

I checked the type: 'transitions' does not actually contain variables, it never will, its constants. self.args is for things that can contain variables.

So we actually don't need to store 'transitions' I think... we can just build self.mapping (and self.levels) and then keep most of the code the same (we just walk over mapping most of the time I assume), and then the reduce code just needs to make sure that removed nodes are removed from self.levels too?

@WoutPiessens
Copy link
Copy Markdown
Collaborator Author

The way I see it is as follows: transitions is only an input argument to the MDD global, and is used to build self.mapping in the constructor. After that, transitions is indeed no longer needed, and it's never used outside of the constructor. self.mapping is the internal representation of the MDD, which changes when calling reduce(). But indeed, removed nodes do need to be removed from self.levels too since that is used outside of the constructor.

Comment thread cpmpy/expressions/globalconstraints.py Outdated
Comment thread cpmpy/expressions/globalconstraints.py Outdated
Comment thread cpmpy/expressions/globalconstraints.py Outdated
Comment thread tests/test_transf_decompose.py Outdated
Copy link
Copy Markdown
Collaborator

@tias tias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great

@tias tias requested a review from IgnaceBleukx May 29, 2026 15:46
Copy link
Copy Markdown
Collaborator

@IgnaceBleukx IgnaceBleukx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, some final small comments that need to be addressed before merging (I'll already approve otherwise I have to re-review...)

Comment thread cpmpy/expressions/globalconstraints.py
Comment thread tests/test_globalconstraints.py Outdated
@tias tias merged commit 3afc737 into master May 29, 2026
12 checks passed
@tias tias deleted the mdd_reduce branch May 29, 2026 18:16
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.

3 participants