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

[WIP] add mass feature to nx.kl_div and harmonize kl computation in the toolbox #654

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

cedricvincentcuaz
Copy link
Collaborator

@cedricvincentcuaz cedricvincentcuaz commented Jul 10, 2024

Types of changes

  • Add feature mass=False|True in nx.kl_div to compute the generalized kl divergence either with the difference of total masses or not.
  • Screened the toolbox to ensure proper use of nx.kl_div and along the way i set all existing offsets to nx.log to 1e-16.

Motivation and context / Related issue

How has this been tested (if it applies)

PR checklist

  • I have read the CONTRIBUTING document.
  • The documentation is up-to-date with the changes I made (check build artifacts).
  • All tests passed, and additional code has been covered with new tests.
  • I have added the PR and Issue fix to the RELEASES.md file.

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.67%. Comparing base (e530985) to head (368f979).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #654   +/-   ##
=======================================
  Coverage   96.66%   96.67%           
=======================================
  Files          85       85           
  Lines       16934    16956   +22     
=======================================
+ Hits        16370    16392   +22     
  Misses        564      564           

@cedricvincentcuaz cedricvincentcuaz merged commit 24ad25c into PythonOT:master Jul 10, 2024
17 checks passed
@mbuze
Copy link

mbuze commented Dec 13, 2024

Hi, this is a fantastic addition! I am currently trying to use sinkhorn_unbalanced2 and would love to have mass=True turned on for the marginal penalisation, but I am struggling to get it to work. Any tips on how to achieve it without much hacking?

(Sorry if I am being confused here -- potentially this does not affect the optimisation routine to find the optimal plan and so it really is just about returning the correct UOT cost?)

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