Conversation
…d config option Co-authored-by: semiexp <7336994+semiexp@users.noreply.github.com>
Co-authored-by: semiexp <7336994+semiexp@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] In
Make domain threshold configurable by adding encoding_domain_threshold config option
Aug 9, 2025
cspuz_core/src/encoder/mod.rs, there is "TODO: make this configurable" comment. Resolve this TODO
semiexp
reviewed
Aug 10, 2025
cspuz_core/src/encoder/mod.rs
Outdated
| if let IntVarRepresentation::Domain(domain) = repr { | ||
| if domain.num_candidates() > 500 && complex_constraints_vars.contains(&var) { | ||
| // TODO: make this configurable | ||
| if domain.num_candidates() > config.encoding_domain_threshold && complex_constraints_vars.contains(&var) { |
Owner
There was a problem hiding this comment.
make it clear that this config is related to log encoding
Contributor
Author
There was a problem hiding this comment.
Fixed in commit a661300. Renamed the config field to log_encoding_domain_threshold and updated CLI option to --log-encoding-domain-threshold to make it clear this config is specifically for log encoding decisions.
semiexp
reviewed
Aug 10, 2025
cspuz_core/src/encoder/mod.rs
Outdated
| let repr = norm_vars.int_var(var); | ||
| let use_direct_encoding = match repr { | ||
| IntVarRepresentation::Domain(domain) => domain.num_candidates() <= 500, | ||
| IntVarRepresentation::Domain(domain) => domain.num_candidates() <= config.encoding_domain_threshold, |
Owner
There was a problem hiding this comment.
this threshold 500 is not related
Contributor
Author
There was a problem hiding this comment.
Fixed in commit a661300. Reverted this threshold back to the hardcoded value of 500, as you're correct - this threshold for direct encoding is conceptually different from the log encoding threshold and should not share the same configuration.
Owner
|
@copilot check two comments |
…ding threshold to hardcoded 500 Co-authored-by: semiexp <7336994+semiexp@users.noreply.github.com>
Copilot
AI
changed the title
Make domain threshold configurable by adding encoding_domain_threshold config option
Make log encoding domain threshold configurable by adding log_encoding_domain_threshold config option
Aug 10, 2025
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.
This PR resolves the TODO comment at line 600 in
cspuz_core/src/encoder/mod.rsby making the hardcoded domain size threshold of 500 configurable for log encoding decisions.Problem
The encoder used a hardcoded value of
500to determine when to use log encoding for variables with large domains in complex constraints. This made it impossible for users to tune the log encoding behavior based on their specific use cases.Solution
Added a new configuration field
log_encoding_domain_thresholdto theConfigstruct with the following changes:log_encoding_domain_threshold: usizefield with default value 500--log-encoding-domain-thresholdcommand-line option with help textconfig.log_encoding_domain_thresholdNote: The threshold at line 690 for direct encoding decisions remains hardcoded at 500, as it serves a different purpose and should not be unified with the log encoding threshold.
Usage
Users can now configure the log encoding threshold via CLI:
Or programmatically:
Backward Compatibility
The default value remains 500, so existing behavior is preserved. All existing tests continue to pass.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.