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

Simplify equalities using their domains #461

Open
1 of 5 tasks
niklasdewally opened this issue Nov 17, 2024 · 5 comments
Open
1 of 5 tasks

Simplify equalities using their domains #461

niklasdewally opened this issue Nov 17, 2024 · 5 comments
Labels
area::conjure-oxide/ast Related to conjure_core and ast representation. area::conjure-oxide/rule-engine Related to the rule engine and the expression rewriting logic. area::conjure-oxide Related to conjure_oxide. area::rules Related to rewrite rules kind::feature New feature or request

Comments

@niklasdewally
Copy link
Collaborator

niklasdewally commented Nov 17, 2024

Description

Implement simplifier rules for = and != based on the domains of their arguments:

  • a = b ~> false if domain(a) ∩ domain(b) = {} (the domains of a and b are completely different)

  • a != b ~> true if domain(a) ∩ domain(b) = {}

  • Do the != case pairwise for an allDiff.

For example, for test mod_undefzero-04-nested-neq, Savile Row does:

Model before undef handling:
language ESSENCE' 1.0
find x : int(5..7)
find y : int(0..3)
find z : int(0..4)
such that
((x%((y%z)))!=3)
Rule:savilerow.TransformMakeSafe
Model has changed. Model after rule application:
language ESSENCE' 1.0
find x : int(5..7)
find y : int(0..3)
find z : int(0..4)
such that
((z != 0) /\ ((((y%z)) != 0) /\ ((x%((y%z)))!=3)))
Model after rule application and simplify:
language ESSENCE' 1.0
find x : int(5..7)
find y : int(0..3)
find z : int(0..4)
such that
(z != 0),
((y%z) != 0)

To do this well, we need the notion of a category, and to cache an expressions category, domain, and type inside its Metadata.

Features

  • Implement domain_of for all expressions #459
  • Implement categories andExpression::category_of.
  • Memoise an expressions category and domain inside Metadata.
  • Implement simplifiers for a=b and a!=b based on the domains of the arguments.
  • Implement an alldiff simplifier based on the domains of the arguments.
  • Refactor rule checks in rules::utils to use categories where possible.

CC: @ozgurakgun

@niklasdewally niklasdewally added kind::feature New feature or request area::conjure-oxide Related to conjure_oxide. area::conjure-oxide/rule-engine Related to the rule engine and the expression rewriting logic. area::conjure-oxide/ast Related to conjure_core and ast representation. labels Nov 17, 2024
@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Nov 17, 2024

don't feel like you have to do this in this PR, in fact you probably shouldn't, but we should first have:

an optional type (this already exists)
an optional domain
an optional category
in the metadata.

we would store the int domain for all Atomic entries, and calculate the domain for all composite expressions.

we can look into how SR calculates bounds to work out the domains. for example for mod and div.

once we calculate the domain of both sides of !=, a rule (maybe the partial eval) should rewrite that expr to true

note: SR's getBounds returns an IntPair but we should maintain a Domain since we will have more kinds of domains, not just ints.

(@ozgurakgun in #456)

@ozgurakgun
Copy link
Contributor

You might want to divorce the alldif one from this plan to make it a bit more manageable. Also remember SR's simplifications for all diff, there is more you can do like pigeonhole analysis: count the number of values and variables you have in the alldiff etc.

The rest sounds good to me!

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Nov 17, 2024

CC: @YehorBoiar @lixitrixi

Having a way to invalidate fields in metadata might affect your current rewriter engine work?

Not sure what you have already, but for example you could call an empty method called Expression::invalidate() / Metadata::invalidate() when an expression has changed, and then whoever takes this on can fill in the memoization logic later?

@lixitrixi
Copy link
Contributor

lixitrixi commented Nov 17, 2024

a = b ~> true if domain(a) / domain(b) = {} (the domains of a and b are identical)

I may be misunderstanding but this seems to only hold when the domains additionally contain just one element? A and B having the same domains doesn't mean they are always equal!

What would make more sense:
a = b ~> true if domain(a) / domain(b) = {} and |domain(a)| = 1

The second one makes sense to me; you could also write an = ~> false version of it.

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Nov 17, 2024

What would make more sense: a = b ~> true if domain(a) / domain(b) = {} and |domain(a)| = 1

The second one makes sense to me; you could also write an = ~> false version of it.

Oops! Checked Savile Row, and it should be a = b ~> false if domain(a) ∩ domain(b) = {}

I've edited the issue

@niklasdewally niklasdewally added the area::rules Related to rewrite rules label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area::conjure-oxide/ast Related to conjure_core and ast representation. area::conjure-oxide/rule-engine Related to the rule engine and the expression rewriting logic. area::conjure-oxide Related to conjure_oxide. area::rules Related to rewrite rules kind::feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants