Skip to content

Commit 2fc624f

Browse files
rsareddy0329Roja Reddy Sareddy
andauthored
fix(rlaif): accept preset reward_prompt template names (#6192)
* fix: update timeout of RLVR sequence_length test * fix: add xdist_group to Nova Bedrock deployment tests * Address rlaif trainer fix * Address rlaif trainer fix --------- Co-authored-by: Roja Reddy Sareddy <rsareddy@amazon.com>
1 parent a248126 commit 2fc624f

2 files changed

Lines changed: 136 additions & 35 deletions

File tree

‎sagemaker-train/src/sagemaker/train/rlaif_trainer.py‎

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class RLAIFTrainer(BaseTrainer):
4949
training_type=TrainingType.LORA,
5050
model_package_group="my-model-group",
5151
reward_model_id="reward-model-id",
52-
reward_prompt="Rate the helpfulness of this response on a scale of 1-10",
52+
reward_prompt="summarize",
5353
training_dataset="s3://bucket/rlaif_data.jsonl"
5454
)
5555
@@ -60,7 +60,7 @@ class RLAIFTrainer(BaseTrainer):
6060
model="meta-llama/Llama-2-7b-hf",
6161
model_package_group="my-rlaif-models",
6262
reward_model_id="reward-model-id",
63-
reward_prompt="Rate the helpfulness of this response on a scale of 1-10"
63+
reward_prompt="summarize"
6464
)
6565
6666
# Create training job (non-blocking)
@@ -396,8 +396,14 @@ def _process_hyperparameters(self):
396396
# Process reward_prompt parameter
397397
if hasattr(self, 'reward_prompt') and self.reward_prompt:
398398
if isinstance(self.reward_prompt, str):
399-
if self.reward_prompt.startswith("Builtin"):
400-
# Handle builtin reward prompts
399+
# Resolution order:
400+
# 1. Preset template name -> resolved locally against the recipe's
401+
# judge_prompt_template enum (no API call). Accepts "Builtin.Summarize",
402+
# "summarize", or "summarize.jinja".
403+
# 2. Evaluator ARN -> validated/assigned as-is.
404+
# 3. Otherwise -> HubContent name lookup (custom registered prompt),
405+
# which raises a clear error if not found.
406+
if self._is_preset_reward_prompt(self.reward_prompt):
401407
self._update_judge_prompt_template_direct(self.reward_prompt)
402408
else:
403409
# Handle evaluator ARN or hub content name
@@ -411,9 +417,44 @@ def _process_hyperparameters(self):
411417
evaluator_arn = _extract_evaluator_arn(self.reward_prompt, "reward_prompt")
412418
self._evaluator_arn = evaluator_arn
413419

420+
@staticmethod
421+
def _normalize_template_name(value: str) -> str:
422+
"""Normalize a preset name or enum path to a comparable key.
423+
424+
Handles an optional "Builtin." prefix, any path prefix, and an optional
425+
".jinja" suffix, case-insensitively. For example "Builtin.Summarize",
426+
"summarize", "summarize.jinja", and "/opt/ml/code/verl/summarize.jinja"
427+
all normalize to "summarize".
428+
"""
429+
name = (value or "").strip()
430+
if name.lower().startswith("builtin."):
431+
name = name.split(".", 1)[1]
432+
name = name.split("/")[-1] # basename
433+
if name.lower().endswith(".jinja"):
434+
name = name[: -len(".jinja")]
435+
return name.lower()
436+
437+
def _get_judge_prompt_template_enum(self):
438+
"""Return the recipe's judge_prompt_template enum values (already in memory)."""
439+
if not self.hyperparameters or not getattr(self.hyperparameters, "_specs", None):
440+
return []
441+
judge_prompt_spec = self.hyperparameters._specs.get("judge_prompt_template", {})
442+
return judge_prompt_spec.get("enum", []) or []
443+
444+
def _is_preset_reward_prompt(self, reward_prompt: str) -> bool:
445+
"""True if reward_prompt matches a recipe preset template (local, no API call).
446+
447+
An explicit "Builtin." prefix always routes to preset resolution so the
448+
user gets a clear "not available" error instead of a HubContent lookup.
449+
"""
450+
if reward_prompt.startswith("Builtin"):
451+
return True
452+
enum_keys = {self._normalize_template_name(e) for e in self._get_judge_prompt_template_enum()}
453+
return self._normalize_template_name(reward_prompt) in enum_keys
454+
414455
def _process_non_builtin_reward_prompt(self):
415-
"""Process non-builtin reward prompt (ARN or hub content name)."""
416-
# Remove judge_prompt_template for non-builtin prompts
456+
"""Process non-preset reward prompt (ARN or hub content name)."""
457+
# Remove judge_prompt_template for non-preset prompts
417458
if hasattr(self.hyperparameters, 'judge_prompt_template'):
418459
delattr(self.hyperparameters, 'judge_prompt_template')
419460
self.hyperparameters._specs.pop('judge_prompt_template', None)
@@ -442,37 +483,42 @@ def _process_non_builtin_reward_prompt(self):
442483

443484

444485
def _update_judge_prompt_template_direct(self, reward_prompt):
445-
"""Update judge_prompt_template based on Builtin reward function."""
486+
"""Resolve a preset reward prompt name to the recipe's judge_prompt_template value.
487+
488+
Accepts "Builtin.Summarize", "summarize", or "summarize.jinja" and matches
489+
it against the recipe's judge_prompt_template enum (normalized by basename,
490+
with an optional ".jinja" suffix). No API call is made.
491+
"""
446492
# Get available templates from hyperparameters specs
447-
judge_prompt_spec = self.hyperparameters._specs.get('judge_prompt_template', {})
448-
available_templates = judge_prompt_spec.get('enum', [])
449-
493+
available_templates = self._get_judge_prompt_template_enum()
494+
450495
if not available_templates:
451496
# If no enum found, use the current value as the only available option
452497
current_value = getattr(self.hyperparameters, 'judge_prompt_template', None)
453498
if current_value:
454499
available_templates = [current_value]
455500
else:
456501
return
457-
458-
# Extract template name after "Builtin." and convert to lowercase
459-
template_name = reward_prompt.split(".", 1)[1].lower()
460-
461-
# Find matching template by extracting filename without extension
502+
503+
# Normalize the requested name (strips optional "Builtin." prefix and ".jinja")
504+
template_name = self._normalize_template_name(reward_prompt)
505+
506+
# Find matching template by normalized basename
462507
matching_template = None
463508
for template in available_templates:
464-
template_filename = template.split("/")[-1].replace(".jinja", "").lower()
465-
if template_filename == template_name:
509+
if self._normalize_template_name(template) == template_name:
466510
matching_template = template
467511
break
468512

469513
if matching_template:
470514
self.hyperparameters.judge_prompt_template = matching_template
471515
else:
472-
available_options = [f"Builtin.{t.split('/')[-1].replace('.jinja', '')}" for t in available_templates]
516+
available_options = [self._normalize_template_name(t) for t in available_templates]
473517
raise ValueError(
474-
f"Selected reward function option '{reward_prompt}' is not available. "
475-
f"Choose one from the available options: {available_options}. "
476-
f"Example: reward_prompt='Builtin.summarize'"
518+
f"Selected reward prompt '{reward_prompt}' is not an available preset. "
519+
f"Choose one from the available options: {available_options} "
520+
f"(pass the name directly, e.g. reward_prompt='{available_options[0]}', "
521+
f"or with the 'Builtin.' prefix). "
522+
f"Alternatively pass an evaluator ARN or a registered HubContent prompt name."
477523
)
478524

‎sagemaker-train/tests/unit/train/test_rlaif_trainer.py‎

Lines changed: 69 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -394,20 +394,23 @@ def test_process_hyperparameters_early_return_on_none(self):
394394
# No exception should be raised
395395

396396
def test_update_judge_prompt_template_direct_with_matching_template(self):
397-
"""Test _update_judge_prompt_template_direct with matching template."""
398-
mock_hyperparams = Mock()
399-
mock_hyperparams._specs = {
400-
'judge_prompt_template': {
401-
'enum': ['templates/summarize.jinja', 'templates/helpfulness.jinja']
397+
"""Test _update_judge_prompt_template_direct resolves Builtin, plain, and .jinja names."""
398+
for reward_prompt in ("Builtin.summarize", "summarize", "summarize.jinja", "Builtin.Summarize"):
399+
mock_hyperparams = Mock()
400+
mock_hyperparams._specs = {
401+
'judge_prompt_template': {
402+
'enum': ['templates/summarize.jinja', 'templates/helpfulness.jinja']
403+
}
402404
}
403-
}
404-
405-
trainer = RLAIFTrainer.__new__(RLAIFTrainer)
406-
trainer.hyperparameters = mock_hyperparams
407-
408-
trainer._update_judge_prompt_template_direct("Builtin.summarize")
409-
410-
assert mock_hyperparams.judge_prompt_template == 'templates/summarize.jinja'
405+
406+
trainer = RLAIFTrainer.__new__(RLAIFTrainer)
407+
trainer.hyperparameters = mock_hyperparams
408+
409+
trainer._update_judge_prompt_template_direct(reward_prompt)
410+
411+
assert mock_hyperparams.judge_prompt_template == 'templates/summarize.jinja', (
412+
f"failed for input {reward_prompt!r}"
413+
)
411414

412415
def test_update_judge_prompt_template_direct_with_no_enum(self):
413416
"""Test _update_judge_prompt_template_direct when no enum is available."""
@@ -434,7 +437,7 @@ def test_update_judge_prompt_template_direct_no_matching_template(self):
434437
trainer = RLAIFTrainer.__new__(RLAIFTrainer)
435438
trainer.hyperparameters = mock_hyperparams
436439

437-
with pytest.raises(ValueError, match="Selected reward function option 'Builtin.nonexistent' is not available"):
440+
with pytest.raises(ValueError, match="Selected reward prompt 'Builtin.nonexistent' is not an available preset"):
438441
trainer._update_judge_prompt_template_direct("Builtin.nonexistent")
439442

440443
def test_update_judge_prompt_template_direct_early_return(self):
@@ -449,6 +452,58 @@ def test_update_judge_prompt_template_direct_early_return(self):
449452
# Should return early without error
450453
trainer._update_judge_prompt_template_direct("Builtin.anything")
451454

455+
def test_normalize_template_name(self):
456+
"""Normalization strips Builtin. prefix, path, and optional .jinja; lowercases."""
457+
cases = {
458+
"summarize": "summarize",
459+
"summarize.jinja": "summarize",
460+
"Builtin.Summarize": "summarize",
461+
"Builtin.summarize.jinja": "summarize",
462+
"/opt/ml/code/verl/summarize.jinja": "summarize",
463+
"bedrock/RLAIF/PandaLM/prompts/grader.jinja": "grader",
464+
" Summarize ": "summarize",
465+
}
466+
for raw, expected in cases.items():
467+
assert RLAIFTrainer._normalize_template_name(raw) == expected, f"failed for {raw!r}"
468+
469+
def test_is_preset_reward_prompt_matches_enum_without_prefix(self):
470+
"""Plain names that match the enum are presets (no API call)."""
471+
mock_hyperparams = Mock()
472+
mock_hyperparams._specs = {
473+
'judge_prompt_template': {
474+
'enum': ['/opt/ml/code/verl/summarize.jinja', 'bedrock/RLAIF/PandaLM/prompts/grader.jinja']
475+
}
476+
}
477+
trainer = RLAIFTrainer.__new__(RLAIFTrainer)
478+
trainer.hyperparameters = mock_hyperparams
479+
480+
assert trainer._is_preset_reward_prompt("summarize") is True
481+
assert trainer._is_preset_reward_prompt("summarize.jinja") is True
482+
assert trainer._is_preset_reward_prompt("Builtin.Summarize") is True
483+
assert trainer._is_preset_reward_prompt("grader") is True
484+
# Builtin.* always routes to preset resolution (for a clear error later)
485+
assert trainer._is_preset_reward_prompt("Builtin.anything") is True
486+
# A raw prompt / unknown name is not a preset -> falls through to ARN/Hub
487+
assert trainer._is_preset_reward_prompt("Rate the helpfulness 1-10") is False
488+
assert trainer._is_preset_reward_prompt("arn:aws:sagemaker:us-east-1:1:evaluator/x") is False
489+
490+
def test_process_hyperparameters_routes_plain_preset_to_template(self):
491+
"""A plain preset name sets judge_prompt_template and never calls Hub."""
492+
mock_hyperparams = Mock()
493+
mock_hyperparams._specs = {
494+
'judge_prompt_template': {'enum': ['/opt/ml/code/verl/summarize.jinja']}
495+
}
496+
trainer = RLAIFTrainer.__new__(RLAIFTrainer)
497+
trainer.hyperparameters = mock_hyperparams
498+
trainer.reward_prompt = "summarize"
499+
trainer.reward_model_id = None
500+
501+
with patch('sagemaker.train.rlaif_trainer._get_hub_content_metadata') as mock_hub:
502+
trainer._process_hyperparameters()
503+
504+
mock_hub.assert_not_called()
505+
assert mock_hyperparams.judge_prompt_template == '/opt/ml/code/verl/summarize.jinja'
506+
452507
def test_process_non_builtin_reward_prompt_removes_judge_template(self):
453508
"""Test _process_non_builtin_reward_prompt removes judge_prompt_template."""
454509
mock_hyperparams = Mock()

0 commit comments

Comments
 (0)