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

Extract function and node specific logic from TD3 #596

Merged
merged 19 commits into from
Nov 30, 2022
Merged

Conversation

sim642
Copy link
Member

@sim642 sim642 commented Feb 15, 2022

Part of issue #545.

This is a quick idea I had to move all the incrementally changed functions stuff out of TD3. The point is to expose separate constraint system incremental data to TD3, which just lists the unknowns to obsolete, delete and reluctantly resolve. These lists are populated by FromSpec itself, which is the last thing that still should know about the specifics of the constraints.
With this change it's actually unnecessary for the solver to even have access to iter_vars from #549 because FromSpec uses it directly to do the conversion.

This is currently on top of #391 although something similar could be done on master as well. Although the annoyance is that it would have to be essentially redone completely there since the changes code in TD3 is changed significantly by #391, particularly by #549.

TODO

  • Remove iter_vars access from solver.
  • Fix server non-relifting in TD3.
  • Benchmark performance change due to intermediate lists?

@sim642 sim642 added the cleanup Refactoring, clean-up label Feb 15, 2022
@sim642 sim642 self-assigned this Feb 16, 2022
@sim642 sim642 added the pr-dependency Depends or builds on another PR, which should be merged before label May 23, 2022
@sim642 sim642 removed the pr-dependency Depends or builds on another PR, which should be merged before label Aug 12, 2022
@sim642 sim642 mentioned this pull request Sep 15, 2022
4 tasks
@sim642 sim642 changed the base branch from interactive to master October 3, 2022 10:55
src/framework/control.ml Fixed Show fixed Hide fixed
@sim642 sim642 requested a review from jerhard October 3, 2022 12:37
@sim642 sim642 marked this pull request as ready for review October 3, 2022 12:37
src/framework/constraints.ml Outdated Show resolved Hide resolved
src/framework/constraints.ml Outdated Show resolved Hide resolved
@sim642 sim642 requested a review from jerhard November 25, 2022 15:47
Copy link
Member

@jerhard jerhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@sim642 sim642 added this to the v2.2.0 milestone Nov 30, 2022
@sim642 sim642 merged commit 33775db into master Nov 30, 2022
@sim642 sim642 deleted the extract-incremental branch November 30, 2022 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Refactoring, clean-up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants