Skip to content

Conversation

@rsareddy0329
Copy link
Contributor

@rsareddy0329 rsareddy0329 commented Dec 16, 2025

Issue #, if available:

Description of changes:

  • Add support to model parameter in evaluator to resolve from BaseTrainer object.
  • Testing:
    • Added unit tests for new BaseTrainer handling
    • Submitted eval job with trainer as input, base_eval_name: mmlu-eval-demo1
      • Benchmark eval(mmlu-eval-demo1-1765923021): arn:aws:sagemaker:us-west-2:729646638167:training-job/pipelines-ayzeo4u9l04n-EvaluateCustomModel-XO0F6U2FMJ
      • Custom scorer(eval-meta-121ce854-1765924799): arn:aws:sagemaker:us-west-2:729646638167:training-job/pipelines-1ufg9wznju1o-EvaluateCustomModel-o2gwjaKdc2
      • LLMAJ eval(eval-meta-e9a13fbc-1765925999): arn:aws:sagemaker:us-west-2:729646638167:training-job/CustomInference-wgvcp7tvxhex-cEW1geWEAA

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rsareddy0329 rsareddy0329 changed the title feat: Add support to trainer object for model parameter in Evaluator feat: Add support to evaluator, Trainer handshake Dec 16, 2025
@rsareddy0329 rsareddy0329 changed the title feat: Add support to evaluator, Trainer handshake feat: Evaluator handshake to trainer Dec 16, 2025
Copy link
Member

Choose a reason for hiding this comment

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

Can we also make type info available here (instead of Any) for all available options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Completed

SAGEMAKER_REGION env var or default region.
sagemaker_session (Optional[Any]): SageMaker session object. If not provided, a default
session will be created automatically.
model (Union[str, Any]): Model for evaluation. Can be:
Copy link
Member

Choose a reason for hiding this comment

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

Need to update the documentation, on additional values allowed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

completed.


try:
# Handle BaseTrainer type
if hasattr(v, '__class__') and v.__class__.__name__ == 'BaseTrainer' or hasattr(v, '_latest_training_job'):
Copy link
Member

Choose a reason for hiding this comment

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

With the conditioning of v.__class__.__name__ == 'BaseTrainer' will I able to supply SFTTrainer?

Copy link
Contributor Author

@rsareddy0329 rsareddy0329 Dec 16, 2025

Choose a reason for hiding this comment

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

Previous jobs were submitted fine, so might have worked.
I moved the code to model_resolution.py to keep it central in this method _resolve_base_model and updated it to isinstance to be sure.

benchmark: _Benchmark
dataset: Union[str, Any] # Required field, must come before optional fields
subtasks: Optional[Union[str, List[str]]] = None
evaluate_base_model: bool = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets make evaluate_base_model false for all evaluations.

@rsareddy0329 rsareddy0329 merged commit 840f3a1 into aws:master Dec 17, 2025
20 of 30 checks passed
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.

3 participants