Add unit tests for pseudo_kl_divergence_loss #59
Conversation
kevinchern
left a comment
kevinchern
left a comment
There was a problem hiding this comment.
Hi Ahmed @abdela47. Thank you for the pull request.
The tests are well-reasoned, modular, and nicely documented.
I did a quick first pass and added some minor requests.
Separately, you may find this contribution guide helpful for our conventions and best-practices.
tests/test_pseudo_kl_divergence.py
Outdated
| logits = torch.zeros(batch, n_spins) | ||
|
|
||
| # spins: (batch_size, n_samples, n_spins) | ||
| spins = torch.ones(batch, n_samples, n_spins) |
There was a problem hiding this comment.
Could you explain your rationale for using zero-valued logits and spins in this test versus nonzero values in the 2d test?
There was a problem hiding this comment.
Zero logits are used in the 3D shape test to keep the entropy term simple and stable (p = 0.5), allowing the test to focus purely on documented shape support; nonzero values are covered in the 2D numerical correctness test.
Co-authored-by: Kevin Chern <[email protected]>
…ins_data.shape for clarity and consistency
There was a problem hiding this comment.
Hi @abdela47, just a couple more changes:
- Can you note this motivation as a comment in the test?
- Can you encapsulate tests in a
unittestframework? And then rename tests to be more concise
thisac
left a comment
thisac
left a comment
There was a problem hiding this comment.
Happy to approve as soon as the final comments from @kevinchern are addressed. Also, please rebase or merge main into branch so that tests can pass (issues have been fixed in main).
|
|
||
|
|
||
| spins_model = torch.ones(batch_size, n_spins, dtype=torch.float32) |
There was a problem hiding this comment.
| spins_model = torch.ones(batch_size, n_spins, dtype=torch.float32) | |
| spins_model = torch.ones(batch_size, n_spins, dtype=torch.float32) |
| entropy2 = F.binary_cross_entropy_with_logits(logits2, probs2) | ||
| (-entropy2).backward() | ||
|
|
||
| torch.testing.assert_close(logits.grad, logits2.grad) |
There was a problem hiding this comment.
Don't forget to add an
if __name__ == "__main__":
unittest.main()at the end of this file to make sure the tests are run correctly.
3 participants
This PR adds deterministic unit tests for pseudo_kl_divergence_loss.
The tests cover both documented spin shapes and verify gradient behavior.
They isolate the statistical structure of the loss using deterministic dummy Boltzmann
machines and do not rely on samplers or quantum hardware.
Closes #56.