Skip to content

Test SamplingProbabilityCorrection layer with different dimensions. #43

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

Merged
merged 1 commit into from
Apr 17, 2025

Conversation

hertschuh
Copy link
Collaborator

The SamplingProbabilityCorrection already supports different dimensions. This adds testing for it.

See #39

The `SamplingProbabilityCorrection` already supports different dimensions. This adds testing for it.

See keras-team#39
Comment on lines +32 to +34
"testcase_name": "logits_rank_2_probs_rank_1",
"logits_rank": 2,
"probs_rank": 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will take a closer look, but shouldn't we ensure that ranks for logits and probs are equal?

That is, logits and ranks can have any dimension, but logits and probs should have equal rank

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a real use case from what I can see.

If your logits have shape [batch_size, num_candidates], you're allowed to pass a probabilities of shape [num_candidates].

The reason, I assume, is that the probability correction is per candidate regardless of the batch index. To not force you to duplicate the probabilities for each batch item, we allow probabilities to have fewer dimensions.

Copy link
Collaborator

@abheesht17 abheesht17 left a comment

Choose a reason for hiding this comment

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

Thanks!

@hertschuh hertschuh merged commit f84ce67 into keras-team:main Apr 17, 2025
5 checks passed
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.

2 participants