Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[bug fix] Torch Classifier agent should call self.backward(loss) #4270

Merged
merged 1 commit into from
Dec 31, 2021

Conversation

dexterju27
Copy link
Contributor

@dexterju27 dexterju27 commented Dec 21, 2021

Patch description
The current torch classifier agent uses loss.backward instead of self.backward(loss). A lot of optimizer behaviors depend on the backward function. While the backward function was ignored in the previous version.
For example:

  1. In SafeFP16Optimizer, the fp32 gradient will be zeros as no synchronizing flag is set. The model doesn't train.
    if self._needs_sync:
    .
  2. In MemoryEfficientFP16Optimizer the loss loss_scale multiplier is omitted. We should improve performance with this fix.
    loss = loss * self.scaler.loss_scale

Testing steps
Train any classifier agent. Verified with a classifier agent training with SafeFP16Optimizer.
In our case, we retrained a safety classifier and compared its performance with the model in the zoo.
FP32 matched the performance, and FP16 performance worse. The performance shifted a bit between the tasks, we are comparing the average class_notok__ f1, it is the validation metric used for all the trainings.

image

@jaseweston jaseweston merged commit 27f22d5 into main Dec 31, 2021
@jaseweston jaseweston deleted the classifier-agent-use-backward branch December 31, 2021 01:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants