QPQ-AG/sophia
10
0
Fork 0
Code Issues 41 Pull Requests 5 Actions Packages Projects Releases 16 Wiki Activity

Improve constraint solving #971

Merged
zxq9 merged 6 commits from improve_constraint_solving into master 2023-08-23 16:43:49 +09:00
Conversation 7 Commits 6 Files Changed 1 +151 -177
zxq9 commented 2023-08-08 21:11:37 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: hanssv

Fingers itched... This solves one actual problem (on Ceres branch once rebased) where three constraints depended on each other and the current implementation only made two iterations.

The rewrite should be more general and easier to further change if necessary. Would be nice to separate solving and error reporting further - but lets save that for another rainy day.

This PR is supported by the Æternity Crypto Foundation

*Created by: hanssv* Fingers itched... This solves one actual problem (on Ceres branch once rebased) where three constraints depended on each other and the current implementation only made two iterations. The rewrite should be more general and easier to further change if necessary. Would be nice to separate solving and error reporting further - but lets save that for another rainy day. This PR is supported by the Æternity Crypto Foundation
zxq9 commented 2023-08-21 04:17:08 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: radrow

I guess this is when we did not improve anything. Why is it ok then?

*Created by: radrow* I guess this is when we did not improve anything. Why is it `ok` then?
zxq9 commented 2023-08-21 04:20:59 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: radrow

Review: Commented

It is hard for me to assess what exactly has changed since many functions have been moved around. Could you add some tests that show the improvement?

Also, it's a great opportunity for adding a bit more comments around...

*Created by: radrow* **Review:** Commented It is hard for me to assess what exactly has changed since many functions have been moved around. Could you add some tests that show the improvement? Also, it's a great opportunity for adding a bit more comments around...
zxq9 commented 2023-08-21 16:18:45 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: hanssv

Feel free to rename it to done or something it doesn't really matter. This is the terminating case in the fix-point recursion.

*Created by: hanssv* Feel free to rename it to `done` or something it doesn't really matter. This is the terminating case in the fix-point recursion.
zxq9 commented 2023-08-21 16:22:35 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: hanssv

@radrow The general problem addressed was the lack of a proper fix-point iteration - this shows up when there are constraints that depend on each other and the order in which you try suddenly matters. It seemed like you or @ghallak had already noticed this -- and "solved" it by adding a second copy of all constraints to the solver.

I could have solved the current issue by adding a third copy of the constraints, but instead opted to implement the iteration that checked for progress more generally. And yes there could have been more comments, but it could also have been done properly the first time.

*Created by: hanssv* @radrow The general problem addressed was the lack of a proper fix-point iteration - this shows up when there are constraints that depend on each other and the order in which you try suddenly matters. It seemed like you or @ghallak had already noticed this -- and "solved" it by adding a _second_ copy of all constraints to the solver. I could have solved the current issue by adding a third copy of the constraints, but instead opted to implement the iteration that checked for progress more generally. And yes there could have been more comments, but it could also have been done properly the first time.
zxq9 commented 2023-08-22 23:00:13 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: radrow

Review: Approved

*Created by: radrow* **Review:** Approved
zxq9 commented 2023-08-23 16:43:36 +09:00 (Migrated from gitlab.com)
Copy Link

Created by: UlfNorell

Review: Approved

*Created by: UlfNorell* **Review:** Approved
zxq9 commented 2023-08-23 16:43:49 +09:00 (Migrated from gitlab.com)
Copy Link

Merged by: hanssv at 2023-08-23 07:43:49 UTC

*Merged by: hanssv at 2023-08-23 07:43:49 UTC*
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
WIP
bug
consensus-breaking
dependencies
documentation
duplicate
effort: high
effort: low
effort: medium
effort: trivial
enhancement
good first issue
help wanted
invalid
maintenance
question
task/feature
todo-in-rewrite
wontfix
bug
Something is not working
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
pig lipstick
Muggle-facing enhancements
question
More information is needed
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: QPQ-AG/sophia#971
No description provided.
Delete Branch "improve_constraint_solving"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?