Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified arcface loss to keep cost function monotonically decreasing #539

Merged
merged 2 commits into from Jan 16, 2023

Conversation

ElisonSherton
Copy link
Contributor

As mentioned in section 2.8 of the old arcface paper under target logit analysis, we can see that whenever the angle between feature vector and target center is too large, the cost function behaviour can change and it wouldn't be monotonically decreasing.

Specifically, when the angle between the feature vector and target center is more obtuse than (180 - margin), the cos function can start to increase.

For instance, consider that my margin is 30 degrees.

If the normed feature vector makes an angle of 130 degrees with the target center, then the logit = cos(130 + 30) = cos(160) = -0.93.

Now consider the normed feature vector makes an angle of 170 degrees with the target center, then the logit = cos(170 + 30) = cos(200) = -0.93.

This doesn't help in our training as we want to penalize highly obtuse angle even more. Hence we make the following change in our loss function as mentioned here to keep this function monotonically decreasing.

Thanks,
Vinayak.

@KevinMusgrave KevinMusgrave changed the base branch from master to dev October 31, 2022 01:27
@KevinMusgrave KevinMusgrave changed the base branch from dev to master October 31, 2022 01:31
@KevinMusgrave KevinMusgrave changed the base branch from master to dev October 31, 2022 01:31
@ElisonSherton
Copy link
Contributor Author

Hi Kevin,

The one test which failed is because the test itself hasn't considered this condition of angle between fv and target center > 180 - margin.

Can I modify the test case for finding correct_loss and raise a pull request again to make this pass?

Thanks,
Vinayak.

@KevinMusgrave
Copy link
Owner

Yes that would be great! Actually you don't need to make another pull request. Just commit your changes to your master branch and they'll appear here.

@ElisonSherton
Copy link
Contributor Author

Yes that would be great! Actually you don't need to make another pull request. Just commit your changes to your master branch and they'll appear here.

HI @KevinMusgrave, I have modified the test_arcface_loss.py test to incorporate the conditional check if angle between target center and fv is less than 180 - margin and handled that case in my master branch. Hope that should trigger these workflows for testing automatically...

Change as follows

FROM

for i, c in enumerate(labels):
    acos = torch.acos(torch.clamp(logits[i, c], -1, 1))
    logits[i, c] = torch.cos(
                acos + torch.tensor(np.radians(margin), dtype=dtype).to(TEST_DEVICE)
          )

TO

for i, c in enumerate(labels):
    acos = torch.acos(torch.clamp(logits[i, c], -1, 1))
    if acos <= (np.pi - np.radians(margin)):
        logits[i, c] = torch.cos(
                        acos + torch.tensor(np.radians(margin), dtype=dtype).to(TEST_DEVICE)
                    )
    else:
        mg = np.radians(margin)
        logits[i, c] -= torch.tensor(mg * np.sin(mg), dtype=dtype).to(TEST_DEVICE)

@KevinMusgrave
Copy link
Owner

Thanks!

@KevinMusgrave
Copy link
Owner

It looks like they got rid of that adjustment: deepinsight/insightface@657ae30

@KevinMusgrave
Copy link
Owner

But it seems reasonable so I'll merge

@KevinMusgrave KevinMusgrave merged commit fafb5c6 into KevinMusgrave:dev Jan 16, 2023
@ElisonSherton
Copy link
Contributor Author

Thanks Kevin!

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.

None yet

2 participants