-
Notifications
You must be signed in to change notification settings - Fork 71
feat: support fast and slow models for SOFAI Sampling Pattern #311
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
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
nrfulton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested some code organization changes. The most important is to not modify Requirement.
mellea/stdlib/sampling/sofai.py
Outdated
| sample_validations=sampled_scores, | ||
| sample_contexts=sample_contexts, | ||
| sample_actions=sampled_actions, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really long method with lots of state. I had quite a lot of trouble following what's happening. Can you please break this up into some private methods that operate independently of one another (except threading through inputs and outputs), and provide a comment block explaining what's going to happen in each step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with new methods:
_run_s1_solver_loop() - S1 iteration loop with validation and repair
_prepare_requirements_for_validation() - Judge backend wrapping logic
_run_s2_solver() - S2 fallback with mode handling
_prepare_s2_context() - Context preparation for each S2 mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. We discussed adding a bit of inline documentation about the for loop during s1.
Please address these issues and let me know when the PR is ready for re-review. Please also note that @jakelorocco will be pushing a slight re-org of the codebase at some point in the coming day or two. This will impact your PR; we will need you to merge with his branch. After removing your changes to Please also install and run pre-commit hooks prior to letting me know this PR as ready for review. I recently added dev environment setup instructions to the Mellea tutorial covering the installation of pre-commit hooks. If you run into issues with your local environment, please ask for guidance here. |
|
@keerthi166 FYI, Jake's changes are in. |
Thanks @nrfulton. We are working on the PR with the Jake's changes. We will be sending PR any minute now. |
c5c8af0 to
6e7b09b
Compare
| # Exit conditions: success returns immediately; no-improvement breaks | ||
| # early to S2 escalation; loop budget exhaustion flows to S2 escalation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect; thanks.
Support for Fast (S1) and Slow (S2) solvers (AI models) in Sampling strategy based on SOFAI cognitive architecture https://sites.google.com/view/sofai/home.