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

Remove SumEq expression #571

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Conversation

niklasdewally
Copy link
Collaborator

@niklasdewally niklasdewally commented Jan 7, 2025

Closes: #515

SumEq was used as an intermediate representation when rewriting sum(...) = e2 into flat sumgeq / sumleq constraints.

Instead, decompose sum(...) = e2 into sum(...) <= e2 /\ sum(...) >= e2 and let the normal Minion process for flattening these convert these to Minion.


Based on PR #570

Chain of upstream PRs as of 2025-01-08

@niklasdewally niklasdewally self-assigned this Jan 7, 2025
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from f1c5421 to c886ff8 Compare January 7, 2025 21:39
@niklasdewally niklasdewally changed the title WIP: ast: add FlatProductEq and Minion conversion rules remove SumEq expression Jan 7, 2025
@niklasdewally niklasdewally changed the title remove SumEq expression Remove SumEq expression Jan 7, 2025
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from c886ff8 to 47dfea4 Compare January 7, 2025 21:42
@niklasdewally niklasdewally marked this pull request as ready for review January 7, 2025 21:43
@niklasdewally niklasdewally linked an issue Jan 7, 2025 that may be closed by this pull request
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from 47dfea4 to a5f7dd8 Compare January 7, 2025 23:19
@ozgurakgun
Copy link
Contributor

This chain of PRs thing at the bottom looks useful (and autogenerated?) What's your workflow that produces those?

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Jan 8, 2025

This chain of PRs thing at the bottom looks useful (and autogenerated?) What's your workflow that produces those?

I am trying out git machete.

Not completely sold on it though...

@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/minion-conversion branch from 08a6f04 to ebecc6b Compare January 8, 2025 12:23
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from a5f7dd8 to fe8c168 Compare January 8, 2025 12:23
@ozgurakgun
Copy link
Contributor

This chain of PRs thing at the bottom looks useful (and autogenerated?) What's your workflow that produces those?

I am trying out git machete.

Not completely sold on it though...

PR chains/stacks are useful though, so might be good. I'll look into it too.

@ozgurakgun
Copy link
Contributor

LGTM, once rebased and tests pass etc.

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Jan 8, 2025

This chain of PRs thing at the bottom looks useful (and autogenerated?) What's your workflow that produces those?

I am trying out git machete.
Not completely sold on it though...

PR chains/stacks are useful though, so might be good. I'll look into it too.

On a similar note, I find doing git rebase --exec 'cargo test' main useful to check that each commit (as opposed to each PR, which is what Github does) has up to date tests. Makes seeing the effects of little rule changes much easier in the history.

@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/minion-conversion branch from ebecc6b to 0206838 Compare January 8, 2025 12:31
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from fe8c168 to f64c2db Compare January 8, 2025 12:31
Base automatically changed from nik/pr/add-mul/minion-conversion to main January 8, 2025 12:32
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from f64c2db to 74403c5 Compare January 8, 2025 12:34
SumEq was used as an intermediate representation when rewriting
`sum(...) = e2` into flat sumgeq / sumleq constraints.

Instead, decompose `sum(...) = e2` into `sum(...) <= e2 /\ sum(...) >=
e2` and let the normal Minion process for flattening these convert these
to Minion.
@niklasdewally niklasdewally force-pushed the nik/pr/add-mul/remove-sum-eq branch from 74403c5 to 4ee053c Compare January 8, 2025 12:53
@niklasdewally niklasdewally merged commit d3ec4ab into main Jan 8, 2025
14 checks passed
@niklasdewally niklasdewally deleted the nik/pr/add-mul/remove-sum-eq branch January 8, 2025 13:05
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.

Remove SumEq
2 participants