-
Notifications
You must be signed in to change notification settings - Fork 28
feat: Semantic Minimisation during Conflict Analysis #406
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
Draft
ImkoMarijnissen
wants to merge
51
commits into
main
Choose a base branch
from
feat/minimisation-during-ca
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
2249e21
Move inference checker to its own file
maartenflippo 32d3af6
Move the deduction checker to `pumpkin_checking`
maartenflippo f8e8080
Build runtime verification into the resolution conflict resolver
maartenflippo 4232ef9
Run deduction verification during tests
maartenflippo c7f3147
Fix CI to actually run deduction checks
maartenflippo e33f843
Implement better printing of failed deduction checks
maartenflippo 74a38a1
Collect initial domains and the last propagation as supporting infere…
maartenflippo b52740b
Do runtime verification of deductions when they are logged
maartenflippo dbd4056
Verify deductions in tests
maartenflippo e46a611
refactor(pumpkin-conflict-resolvers): Correctly log domain inferences…
maartenflippo 9b9e7ee
fix(pumpkin-conflict-resolvers): Log inference for freshly added fals…
maartenflippo c1565e1
Fix docs for atomic! macro
maartenflippo 5064db2
Apply fixes based on feedback in review
maartenflippo 18aa665
Correctly log inferences for minimisation
maartenflippo 56e5a14
Remove redundant inferences introduced through minimisation
maartenflippo e1c9728
Do not disable recursive minimisation when proof logging
maartenflippo 5be0213
Fix compile error
maartenflippo 0194f65
Work around assumptions by introducing them as initial domains
maartenflippo 9f3cc2f
Allow inconsistent premises in the deduction
maartenflippo ce38731
feat: setup for removing elements during conflict analysis
ImkoMarijnissen 69e6e7c
feat: adding statistics
ImkoMarijnissen a6cdcb4
fix: time-table checker not passing with holes
ImkoMarijnissen 0fd347b
Merge branch 'fix/time-table-checker' into feat/minimisation-during-ca
ImkoMarijnissen b99b962
fix: swapping around of values
ImkoMarijnissen 3f99bf2
Merge branch 'fix/time-table-checker' into feat/minimisation-during-ca
ImkoMarijnissen 2c70913
Revert "fix: swapping around of values"
ImkoMarijnissen a34897a
fix: remove instead of add
ImkoMarijnissen ad663ad
Merge branch 'fix/time-table-checker' into feat/minimisation-during-ca
ImkoMarijnissen 954aba4
feat: replacing predicates with new predicates + improving statistic …
ImkoMarijnissen 376f4da
Merge branch 'main' into feat/minimisation-during-ca
ImkoMarijnissen 3df8f04
feat: add flag for running with iterative minimisation
ImkoMarijnissen 49b50f2
chore: left over files
ImkoMarijnissen fefee6a
feat: initial attempt at making minimisation during ca more efficient
ImkoMarijnissen e737f83
WIP: making progress
ImkoMarijnissen ec13f55
fix: logging root-level deductions when inferring redundancy
ImkoMarijnissen 787449b
fix: logging more root-level inferences + remove predicate from itera…
ImkoMarijnissen d2f0739
Merge branch 'main' into feat/minimisation-during-ca
ImkoMarijnissen ac03d34
chore: clippy warnings
ImkoMarijnissen 479b5e4
chore: clippy warnings duplicate crate
ImkoMarijnissen 7de5ba3
docs: adding documentation
ImkoMarijnissen f95f127
fix: only log in proof when necessary
ImkoMarijnissen 607659d
chore: remove unnecessary logging of unset parameters
ImkoMarijnissen 61f2253
fix: hole update after removing lower or upper-bound
ImkoMarijnissen 081a5c7
docs: adding documentation to iterative minimiser
ImkoMarijnissen 699c6af
refactor: changing to variable state
ImkoMarijnissen 0cc079b
refactor: allow removing of multiple elements at once
ImkoMarijnissen 99c1150
refactor: do not allocate or reset when unnecessary
ImkoMarijnissen 606834c
refactor: also remove not equals when introducing lower-bound
ImkoMarijnissen e06823d
fix: issue with root bounds + issue with removing not actually removing
ImkoMarijnissen a003524
fix: argument to pumpkin solver
ImkoMarijnissen 18ac726
Merge branch 'main' into feat/minimisation-during-ca
ImkoMarijnissen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| allowed-duplicate-crates = [ | ||
| "wit-bindgen", | ||
| "regex-automata", | ||
| "anstream", | ||
| "anstyle-parse", | ||
| "regex-syntax", | ||
| ] |
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
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
295 changes: 295 additions & 0 deletions
295
pumpkin-crates/conflict-resolvers/src/minimisers/iterative_minimiser.rs
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,295 @@ | ||
| use pumpkin_checking::VariableState; | ||
| use pumpkin_core::conflict_resolving::ConflictAnalysisContext; | ||
| use pumpkin_core::containers::KeyedVec; | ||
| use pumpkin_core::containers::StorageKey; | ||
| use pumpkin_core::predicate; | ||
| use pumpkin_core::predicates::Predicate; | ||
| use pumpkin_core::predicates::PredicateType; | ||
| use pumpkin_core::propagation::ReadDomains; | ||
| use pumpkin_core::variables::DomainId; | ||
|
|
||
| /// A minimiser which iteratively applies rewrite rules based on the semantic meaning of predicates | ||
| /// *during* conflict analysis. | ||
| /// | ||
| /// The implementation is heavily inspired by \[1\]. | ||
| /// | ||
| /// There are a couple of limitations to note: | ||
| /// - Currently, whenever a hole is created at a lower- or upper-bound, the bound is only moved by 1 | ||
| /// (even if there are multiple holes which could cause a higher lower-bound). This simplifies a | ||
| /// lot of the logic. | ||
| /// - Currently, it is unclear how the rules are applied in \[1\]. This means that certain rules are | ||
| /// invalid (for, as of yet, unknown reasons), for example, removing all other elements concerning | ||
| /// `x` when `[x = v]` is added leads to wrong solutions. | ||
| /// - This leads to another slightly nuanced point; which is that when an element is removed from | ||
| /// the nogood, it could be that there are other bounds which should be restored. | ||
| /// | ||
| /// For example, it could be that the nogood contains (among others) the predicates `[x >= v]` | ||
| /// and `[x = v + 1]`. In this case, once `[x = v + 1]` gets removed, the lower-bound should be | ||
| /// moved back to `v`. | ||
| /// | ||
| /// ## Developer Notes | ||
| /// - The predicates from the previous decision level should also be added to the | ||
| /// [`IterativeMinimiser`]; this is due to the fact that they are not guaranteed to be | ||
| /// semantically minimised away. | ||
| /// | ||
| /// Imagine the situation where we have `[x >= v]` from a previous | ||
| /// decision level and `[x >= v']` from the current decision level (where `v' > v`). If we then | ||
| /// do not process `[x >= v]`, then it would get added to the nogood directly rather than removed | ||
| /// due to redundancy. If we now resolve on [`x >= v'`] and the nogood becomes asserting, then | ||
| /// there are no other elements over `x` and the predicate from the previous decision level does | ||
| /// not get removed. | ||
| /// | ||
| /// # Bibliography | ||
| /// \[1\] T. Feydy, A. Schutt, and P. Stuckey, ‘Semantic learning for lazy clause generation’, in | ||
| /// TRICS workshop, held alongside CP, 2013. | ||
| #[derive(Debug, Clone, Default)] | ||
| pub(crate) struct IterativeMinimiser { | ||
| /// Keeps track of the domains induced by the current working nogood. | ||
| state: VariableState<Predicate>, | ||
| domains: KeyedVec<DomainId, Vec<Predicate>>, | ||
| root_predicates: KeyedVec<DomainId, Vec<Predicate>>, | ||
| } | ||
|
|
||
| /// The result of processing a predicate, indicating its redundancy. | ||
| #[derive(Debug, Clone)] | ||
| pub(crate) enum ProcessingResult { | ||
| /// The predicate to process was redundant. | ||
| Redundant, | ||
| /// The predicate to process was not redundant, and it replaced | ||
| /// [`ProcessingResult::ReplacedPresent::removed`]. | ||
| /// | ||
| /// e.g., [x >= 5] can replace [x >= 2]. | ||
| ReplacedPresent { removed: Vec<Predicate> }, | ||
| /// The predicate to process was replaced with | ||
| /// [`ProcessingResult::PossiblyReplacedWithNew::new_predicate`] and it also removed | ||
| /// [`ProcessingResult::PossiblyReplacedWithNew::removed`]. | ||
| /// | ||
| /// Note that it is not always possible to replace with `new_predicate` (since it can lead to | ||
| /// infinite loops), so it is not guaranteed that `new_predicate` is added. | ||
| /// | ||
| /// e.g., if [x >= 5] is in the nogood, and the predicate [x <= 5] is added, then [x >= 5] is | ||
| /// removed and replaced with [x == 5] (and [x <= 5] is not added). | ||
| PossiblyReplacedWithNew { | ||
| removed: Predicate, | ||
| new_predicate: Predicate, | ||
| }, | ||
| /// The predicate was found to be not redundant. | ||
| NotRedundant, | ||
| } | ||
|
|
||
| impl IterativeMinimiser { | ||
| /// Removes the given predicate from the nogood. | ||
| pub(crate) fn remove_predicate(&mut self, predicate: Predicate) { | ||
| let domain = predicate.get_domain(); | ||
| while let Some(to_remove_position) = self.domains[domain] | ||
| .iter() | ||
| .position(|element| *element == predicate) | ||
| { | ||
| let _ = self.domains[domain].remove(to_remove_position); | ||
| } | ||
| } | ||
|
|
||
| /// Applies the given predicate from the nogood. | ||
| pub(crate) fn apply_predicate( | ||
| &mut self, | ||
| predicate: Predicate, | ||
| context: &ConflictAnalysisContext, | ||
| ) { | ||
| // println!("APPLYING {predicate:?}"); | ||
| let domain = predicate.get_domain(); | ||
| self.domains.accomodate(domain, Default::default()); | ||
| self.domains[domain].push(predicate); | ||
|
|
||
| if context.is_proof_logging_inferences() | ||
| && context.get_checkpoint_for_predicate(predicate) == Some(0) | ||
| { | ||
| self.root_predicates.accomodate(domain, Default::default()); | ||
| self.root_predicates[domain].push(predicate) | ||
| } | ||
| } | ||
|
|
||
| /// Explains the lower-bound in the proof log. | ||
| fn explain_lower_bound_in_proof( | ||
| &self, | ||
| predicate: Predicate, | ||
| context: &mut ConflictAnalysisContext, | ||
| ) { | ||
| if context.is_proof_logging_inferences() { | ||
| let domain = predicate.get_domain(); | ||
|
|
||
| if domain.index() >= self.root_predicates.len() { | ||
| return; | ||
| } | ||
|
|
||
| for root_predicate in self.root_predicates[domain] | ||
| .iter() | ||
| .filter(|root_predicate| { | ||
| root_predicate.is_lower_bound_predicate() | ||
| || root_predicate.is_not_equal_predicate() | ||
| }) | ||
| { | ||
| context.explain_root_assignment(*root_predicate); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn explain_upper_bound_in_proof( | ||
| &self, | ||
| predicate: Predicate, | ||
| context: &mut ConflictAnalysisContext, | ||
| ) { | ||
| if context.is_proof_logging_inferences() { | ||
| let domain = predicate.get_domain(); | ||
|
|
||
| if domain.index() >= self.root_predicates.len() { | ||
| return; | ||
| } | ||
|
|
||
| for root_predicate in self.root_predicates[domain] | ||
| .iter() | ||
| .filter(|root_predicate| { | ||
| root_predicate.is_upper_bound_predicate() | ||
| || root_predicate.is_not_equal_predicate() | ||
| }) | ||
| { | ||
| context.explain_root_assignment(*root_predicate); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Processes the predicate, indicating via [`ProcessingResult`] what can happen to it. | ||
| pub(crate) fn process_predicate( | ||
| &mut self, | ||
| predicate: Predicate, | ||
| context: &mut ConflictAnalysisContext, | ||
| ) -> ProcessingResult { | ||
| let domain = predicate.get_domain(); | ||
| if domain.index() >= self.domains.len() || self.domains[domain].is_empty() { | ||
| return ProcessingResult::NotRedundant; | ||
| } | ||
| self.domains.accomodate(domain, Default::default()); | ||
|
|
||
| self.state.reset_domain(domain); | ||
| for predicate in self.domains[predicate.get_domain()].iter() { | ||
| let consistent = self.state.apply(predicate); | ||
| assert!(consistent) | ||
| } | ||
|
|
||
| let lower_bound = self | ||
| .state | ||
| .lower_bound(&predicate.get_domain()) | ||
| .try_into() | ||
| .unwrap_or(i32::MIN); | ||
| let upper_bound = self | ||
| .state | ||
| .upper_bound(&predicate.get_domain()) | ||
| .try_into() | ||
| .unwrap_or(i32::MAX); | ||
|
|
||
| // If the domain is assigned, then the added predicate is redundant. | ||
| // | ||
| // Encompasses the rules: | ||
| // - [x = v], [x != v'] => [x = v] | ||
| // - [x = v], [x <= v'] => [x = v] | ||
| // - [x = v], [x >= v'] => [x = v] | ||
| if lower_bound == upper_bound { | ||
| self.explain_lower_bound_in_proof(predicate, context); | ||
| self.explain_upper_bound_in_proof(predicate, context); | ||
| return ProcessingResult::Redundant; | ||
| } | ||
|
|
||
| match predicate.get_predicate_type() { | ||
| PredicateType::LowerBound => { | ||
| if predicate.get_right_hand_side() == upper_bound { | ||
| self.explain_upper_bound_in_proof(predicate, context); | ||
| // [x <= v], [x >= v] => [x = v] | ||
| ProcessingResult::PossiblyReplacedWithNew { | ||
| removed: predicate!(domain <= upper_bound), | ||
| new_predicate: predicate!(domain == upper_bound), | ||
| } | ||
| } else if predicate.get_right_hand_side() > lower_bound { | ||
| // [x >= v], [x >= v'] => [x >= v'] if v' > v | ||
| let to_remove = self.domains[predicate.get_domain()] | ||
| .iter() | ||
| .filter(|element| { | ||
| element.is_lower_bound_predicate() | ||
| || (element.is_not_equal_predicate() | ||
| && element.get_right_hand_side() | ||
| < predicate.get_right_hand_side()) | ||
| }) | ||
| .copied() | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| if !to_remove.is_empty() { | ||
| ProcessingResult::ReplacedPresent { removed: to_remove } | ||
| } else { | ||
| ProcessingResult::NotRedundant | ||
| } | ||
| } else { | ||
| // [x >= v], [x >= v'] => [x >= v] if v > v' | ||
| self.explain_lower_bound_in_proof(predicate, context); | ||
| ProcessingResult::Redundant | ||
| } | ||
| } | ||
| PredicateType::UpperBound => { | ||
| // [x >= v], [x <= v] => [x = v] | ||
| if predicate.get_right_hand_side() == lower_bound { | ||
| self.explain_lower_bound_in_proof(predicate, context); | ||
| ProcessingResult::PossiblyReplacedWithNew { | ||
| removed: predicate!(domain >= lower_bound), | ||
| new_predicate: predicate!(domain == lower_bound), | ||
| } | ||
| } else if predicate.get_right_hand_side() < upper_bound { | ||
| // [x <= v], [x <= v'] => [x <= v'] if v' < v | ||
| let to_remove = self.domains[predicate.get_domain()] | ||
| .iter() | ||
| .filter(|element| { | ||
| element.is_upper_bound_predicate() | ||
| || (element.is_not_equal_predicate() | ||
| && element.get_right_hand_side() | ||
| > predicate.get_right_hand_side()) | ||
| }) | ||
| .copied() | ||
| .collect::<Vec<_>>(); | ||
| if !to_remove.is_empty() { | ||
| ProcessingResult::ReplacedPresent { removed: to_remove } | ||
| } else { | ||
| ProcessingResult::NotRedundant | ||
| } | ||
| } else { | ||
| // [x <= v], [x <= v'] => [x <= v] if v < v' | ||
| self.explain_upper_bound_in_proof(predicate, context); | ||
| ProcessingResult::Redundant | ||
| } | ||
| } | ||
| PredicateType::NotEqual => { | ||
| if predicate.get_right_hand_side() == upper_bound { | ||
| // [x <= v], [x != v] => [x <= v - 1] | ||
| self.explain_upper_bound_in_proof(predicate, context); | ||
| ProcessingResult::PossiblyReplacedWithNew { | ||
| removed: predicate!(domain <= upper_bound), | ||
| new_predicate: predicate!(domain <= upper_bound - 1), | ||
| } | ||
| } else if predicate.get_right_hand_side() > upper_bound { | ||
| // [x <= v], [x != v'] => [x <= v] where v' > v | ||
| self.explain_upper_bound_in_proof(predicate, context); | ||
| ProcessingResult::Redundant | ||
| } else if predicate.get_right_hand_side() == lower_bound { | ||
| // [x >= v], [x != v] => [x <= v + 1] | ||
| self.explain_lower_bound_in_proof(predicate, context); | ||
| ProcessingResult::PossiblyReplacedWithNew { | ||
| removed: predicate!(domain >= lower_bound), | ||
| new_predicate: predicate!(domain >= lower_bound + 1), | ||
| } | ||
| } else if predicate.get_right_hand_side() < lower_bound { | ||
| // [x >= v], [x != v'] => [x >= v] where v' < v | ||
| self.explain_lower_bound_in_proof(predicate, context); | ||
| ProcessingResult::Redundant | ||
| } else { | ||
| ProcessingResult::NotRedundant | ||
| } | ||
| } | ||
| PredicateType::Equal => ProcessingResult::NotRedundant, | ||
| } | ||
| } | ||
| } | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.