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

Generic LR cost decomposition #99

Merged
merged 15 commits into from
Jul 12, 2022

Conversation

michalk8
Copy link
Collaborator

@michalk8 michalk8 commented Jul 5, 2022

In this PR:

  • add decomposition of generic cost matrices
  • allow factorizing generic cost matrices in LR Gromov-Wasserstein, not only sqeucl point clouds
  • fix missing ott import in GWLRSinkhorn.ipynb
  • default to use_danskin=False in LROutput.compute_reg_ot_cost (same as in LRSinkhornState); fixes LRSinkhorn.ipynb

@michalk8 michalk8 added the enhancement New feature or request label Jul 5, 2022
@michalk8 michalk8 self-assigned this Jul 5, 2022
Copy link
Contributor

@marcocuturi marcocuturi left a comment

Choose a reason for hiding this comment

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

This is great. I am guessing you plan to add a test for that functionality.

ott/geometry/geometry.py Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2022

Codecov Report

Merging #99 (69743d0) into main (5d48391) will increase coverage by 1.18%.
The diff coverage is 89.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #99      +/-   ##
==========================================
+ Coverage   80.99%   82.17%   +1.18%     
==========================================
  Files          41       41              
  Lines        3630     3708      +78     
  Branches      423      431       +8     
==========================================
+ Hits         2940     3047     +107     
+ Misses        568      532      -36     
- Partials      122      129       +7     
Impacted Files Coverage Δ
ott/core/was_solver.py 95.00% <ø> (ø)
ott/core/gromov_wasserstein.py 85.71% <76.47%> (+2.52%) ⬆️
ott/geometry/low_rank.py 93.85% <80.00%> (-1.34%) ⬇️
ott/geometry/geometry.py 78.27% <90.19%> (+2.89%) ⬆️
ott/core/sinkhorn_lr.py 91.70% <100.00%> (ø)
ott/geometry/grid.py 90.26% <100.00%> (+0.08%) ⬆️
ott/geometry/pointcloud.py 79.41% <100.00%> (+12.40%) ⬆️

@michalk8 michalk8 marked this pull request as ready for review July 11, 2022 15:14
@michalk8 michalk8 requested a review from marcocuturi July 11, 2022 15:15
Copy link
Contributor

@marcocuturi marcocuturi left a comment

Choose a reason for hiding this comment

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

Thanks Michal! this is really great, i hope we can find nice applications with this.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@michalk8 michalk8 merged commit 79486ce into ott-jax:main Jul 12, 2022
@michalk8 michalk8 deleted the feature/generic-lr-cost-factorization branch July 12, 2022 12:46
@michalk8 michalk8 mentioned this pull request Jul 12, 2022
michalk8 added a commit that referenced this pull request Jun 27, 2024
* Initial implementation of generic LR cost decomp

* Add subset method

* Annotate array sizes, use multi_dot

* [ci skip] Make `to_LRCGeometry` in LR geom no-op

* Fix ``to_LRCGeometry`` when online, update docs

* Add factorization tests

* Add test for subsetting

* Polish documentation, add bibtex

* Fix unnecessary indents

* Disable `pytest-xdist` for all tests on CI

* Update GW to include generic LR cost decomp

* Fix LR cost conversion check in GW, add  test

* Fix `{GW,}LR` tutorial, use_danskin=False in LROut
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants