FIX/refactor: centralize CP MiniZinc model assembly#445
Merged
Conversation
Co-authored-by: Copilot <copilot@github.com>
|
peacker
approved these changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



CP MiniZinc Model Parts Standardization
CLAASP has many CP model generators. These generators turn MiniZinc fragments into full MiniZinc models. The fragments come from several sources:
Currently there are several structural problems.
1. Fragment Names Are Not Consistent
Different models use different attributes for the same conceptual fragment.
Example: in
claasp/cipher_modules/models/cp/mzn_models/mzn_xor_differential_trail_search_fixing_number_of_active_sboxes_model.py, the first-step model stores generated constraints in:But the second-step model stores generated constraints in:
So the same kind of fragment, MiniZinc constraints, is stored under two different names depending on the sub-model.
A clearer design would use explicit model parts:
2.
_model_prefixHas Multiple MeaningsMany models use:
but it does not always mean only "prefix".
In the base model, it starts as MiniZinc includes and helper predicates/functions:
But some models also add declarations to it.
Example from
mzn_xor_differential_model.py:Here
variablescontains declarations such as:So
_model_prefixbecomes:A cleaner contract would be:
3. Some Attributes Change Meaning During Build
Some attributes start as one kind of fragment and later become a full MiniZinc model.
Example from the first-step model:
At this point:
means:
Later:
Now:
means:
The same happens with
_model_constraints.Example from
mzn_cipher_model.py:At this point it means constraints only.
Later:
Now it means:
4. Model Assembly Order Is Not Uniform
Different models assemble MiniZinc text in different ways.
First-step model example from
mzn_xor_differential_number_of_active_sboxes_model.py:Order:
Other models copy only the prefix into
_model_constraints.Example from
mzn_cipher_model.py:Example from
mzn_xor_differential_model.py:Example from
mzn_xor_linear_model.py:Order:
Some solve paths then assemble with:
For example, in the base
solve()path:This works only because many builders already copied the prefix into
_model_constraints.That makes the final MiniZinc model dependent on hidden assumptions about whether
_model_constraintsis raw constraints or already a full model.A cleaner design would use one assembly method:
5. Two-Step Models Are Multiple Self-Contained Models
The two-step flow is not one model. It has at least:
Currently the first one is stored in:
and the second one is stored using the normal model fields:
But both are self-contained MiniZinc models.
A clearer representation would be:
Then both models follow the same structure:
The intention of this PR is only to introduce MiniZincModelParts as the new way to manage MiniZinc model parts. This class provides the assemble_model flow, which for now preserves the existing behavior even when the current fragments still mix variable declarations with constraints, or declarations with includes.
In other words, this PR does not fully reorganize every CP model yet. Its goal is to introduce the structure and give reviewers a clear view of the new direction for managing MiniZinc fragments. In the next PR, we will inspect the models one by one and move each fragment into the correct part.
Note:
The issues described above were revealed and confirmed while working on PR #444: #444. That PR aims to ensure CP models do not include MiniZinc predicates/functions that they do not actually use.