Repository navigation
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
While adding
old_log_probsis the correct fix for theTypeErrormentioned in the PR description, the subsequent call toadv_estimator_fn(**adv_kwargs)on line 221 will still raise aTypeError.The
adv_kwargsdictionary contains keys likeconfig(and potentiallyreward_baselines) which are not accepted by the target functionscompute_optimal_token_baseline_advantageandcompute_multi_turn_optimal_token_baseline_advantageinverl/trainer/ppo/core_algos.py. These functions are missing a**kwargsparameter to accept unused arguments, unlike other advantage estimators in the same file.To fully fix the OTB feature, you should modify the signatures of those two functions in
verl/trainer/ppo/core_algos.pyto accept arbitrary keyword arguments. For example:A similar change is needed for
compute_multi_turn_optimal_token_baseline_advantage. This will prevent the newTypeErrorand make the OTB feature fully functional.