[algo] refactor: don't special-case compute_policy_loss

[frrad]

What does this PR do?

currently the vanilla policy loss mode is special cased. this moves vanilla onto the shared interface and stops speical-casing it.

Checklist Before Submitting

Important

Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.

• [X ] Read the Contribute Guide.
• [x ] Apply pre-commit checks: pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always
• [ x] Add / Update the documentation.
• [ x] Add unit or end-to-end test(s) to the CI workflow to cover all the code. If not feasible, explain why: ...
• [x ] Once your PR is ready for CI, send a message in the ci-request channel in the verl Slack workspace. (If not accessible, please try the Feishu group (飞书群).)

[gemini-code-assist]

Code Review

This pull request refactors the policy loss computation by removing the special case for the 'vanilla' mode. The vanilla policy loss is now registered and accessed through the same shared interface as other loss functions, which improves modularity and reduces code duplication in the actor workers.

My review focuses on the new shared interface. I've identified a couple of high-severity issues related to incorrect optional type hinting and a redundant assertion in the new compute_policy_loss_vanilla function. Addressing these will improve code clarity, maintainability, and static analysis correctness.

[gemini-code-assist]

The type hint for the config parameter is Optional[AlgoConfig], but this appears to be incorrect. The function accesses attributes like config.clip_ratio, which are defined in the AlgoConfig class. The config object passed from the callers in dp_actor.py and megatron_actor.py is the actor's configuration, which contains the algorithm's configuration.

Additionally, the parameter is marked as Optional with a default of None, but it's treated as mandatory within the function (see the assertion on line 750). Since callers always provide this configuration, it's better to make it a required, non-optional parameter.

Suggested change

	def compute_policy_loss_vanilla(
	old_log_prob: torch.Tensor,
	log_prob: torch.Tensor,
	advantages: torch.Tensor,
	response_mask: torch.Tensor,
	loss_agg_mode: str = "token-mean",
	):
	config: Optional[AlgoConfig] = None,
	) -> tuple[torch.Tensor, torch.Tensor, torch.Tensor, torch.Tensor]:
	def compute_policy_loss_vanilla(
	old_log_prob: torch.Tensor,
	log_prob: torch.Tensor,
	advantages: torch.Tensor,
	response_mask: torch.Tensor,
	loss_agg_mode: str = "token-mean",
	config: AlgoConfig,
	) -> tuple[torch.Tensor, torch.Tensor, torch.Tensor, torch.Tensor]:

[gemini-code-assist]

This assertion is redundant if the config parameter in the function signature is made non-optional, as suggested in a separate comment. Since callers always provide the configuration, making the parameter mandatory in the signature is a cleaner approach and removes the need for this runtime check.

[zhaochenyang20]

@frederrx let me help on the merge! Thanks!

[eric-haibin-lin]

despite that verl does not list compute_policy_loss as an official API, we do notice that many verl extensions import this function in their recipe. I'd recommend mark it with verl.utils.import_utils.deprecated, add a new function compute_policy_loss_vanilla, note it in #2744. We then remove compute_policy_loss in the next release.

[frrad]

changed. ptal