Skip to content

Conversation

@Sibylau
Copy link
Contributor

@Sibylau Sibylau commented Sep 18, 2025

Stacked PRs:


[Benchmark] kl_div kernel and test

Sibylau added a commit that referenced this pull request Sep 18, 2025
stack-info: PR: #615, branch: Sibylau/stack/1
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 18, 2025
elif reduction == "mean":
final_loss = torch.sum(loss) / (BT * V)
else: # reduction == "none"
final_loss = loss
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we test all reductions in the unit tests / main function? or maybe simplify it to just one case that's being used by tritonbench?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tritonbench only tests the reduction = "batchmean" case: https://github.com/meta-pytorch/tritonbench/blob/main/tritonbench/operators/kl_div/operator.py#L28
while liger_kernel implements all cases. I mirrored liger_kernel implementation: https://github.com/linkedin/Liger-Kernel/blob/main/src/liger_kernel/ops/kl_div.py#L150

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you suggest I add all unit tests for all reductions, or only implement reduction = "batchmean" ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, let's just keep it as-is now.

@Sibylau
Copy link
Contributor Author

Sibylau commented Sep 18, 2025

codegen test passes python -m unittest test_examples.TestExamples.test_kl_div
image
benchmark accuracy passes python benchmarks/run.py --metrics accuracy --kernel kl_div
image

Copy link
Contributor

@yf225 yf225 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @Sibylau ! rebasing should clear the CI errors

@yf225
Copy link
Contributor

yf225 commented Sep 18, 2025

I took the liberty to rebase the PR. Will merge it after the CI clears.

@yf225 yf225 merged commit a70562a into main Sep 18, 2025
13 checks passed
@yf225 yf225 deleted the Sibylau/stack/1 branch September 18, 2025 04:30
lolpack pushed a commit to lolpack/helion that referenced this pull request Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants