Skip to content

feat(eval): add missing minimax backend configuration#86

Open
Tanmay9223 wants to merge 1 commit into
microsoft:mainfrom
Tanmay9223:fix-eval-qwen-minimax
Open

feat(eval): add missing minimax backend configuration#86
Tanmay9223 wants to merge 1 commit into
microsoft:mainfrom
Tanmay9223:fix-eval-qwen-minimax

Conversation

@Tanmay9223

@Tanmay9223 Tanmay9223 commented Jun 24, 2026

Copy link
Copy Markdown

🎯 What: Adds missing configuration setup in scripts/eval_only.py to properly support the minimax_chat backend.
💡 Why: skillopt-eval was failing to set up external backend calls for minimax correctly because the invocation to configure_minimax_chat was entirely missing, unlike in skillopt-train. This covers the edge cases not addressed by PR #85.
Verification: Verified locally that running scripts/eval_only.py with backend: minimax parses arguments and configures the environment without crashing.
Result: Users can now seamlessly evaluate their models with the MiniMax backend using skillopt-eval.

@Tanmay9223

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Add missing configuration setup in scripts/eval_only.py to properly
support the minimax_chat backend, which was entirely omitted.

Fix the following coverage gaps in eval_only.py:
- Add minimax CLI arguments
- Include the minimax config mappings in _MAP
- Update the backend parsing logic
- Call configure_minimax_chat
@Tanmay9223 Tanmay9223 force-pushed the fix-eval-qwen-minimax branch from 914f1bf to f1f21e6 Compare June 24, 2026 18:14
@Tanmay9223 Tanmay9223 changed the title fix(eval): correctly setup calls to qwen and minimax backends feat(eval): add missing minimax backend configuration Jun 24, 2026
@Yif-Yang

Copy link
Copy Markdown
Contributor

Thanks for this @Tanmay9223! 🙏

Heads up on coordination: the eval-only backend wiring just landed via #85 (qwen + minimax configure_* calls in scripts/eval_only.py), and the MiniMax half of this PR overlaps almost entirely with what's now on main.

Comparing the two, #87 is effectively a superset of this PR — it carries the same eval_only.py minimax change plus the same json_utils.py change plus the new sleep verifier test. To avoid landing the same diff twice and creating a three-way conflict in eval_only.py, I'd suggest we close this one in favor of #87 and consolidate the review there.

Could you confirm you're OK closing #86? If there's anything in here that isn't also in #87, let me know and we'll make sure it's preserved. Really appreciate the backend coverage work.

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.

2 participants