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

Fix/padded scaling #116

Merged
merged 44 commits into from
Aug 3, 2022
Merged

Fix/padded scaling #116

merged 44 commits into from
Aug 3, 2022

Conversation

michalk8
Copy link
Collaborator

@michalk8 michalk8 commented Jul 25, 2022

In this PR:

  • removes scale_cost=None, default is scale_cost=1.0
  • allows scale_cost to also be int, not only float
  • makes private the following Geometry functions:
    • compute_cost_matrix -> _compute_cost_matrix(), to avoid confusion between cost_matrix property
    • compute_summary_online -> _compute_summary_online()
    • leading_slice -> _leading_slice(), technical detail for batching
  • removes rescale_cost_fn, unused
  • add {src,tgt}_mask for geometries that are used to compute statistics of cost matrices, such as mean/median/inv. scaling; useful when padding geomeries in segment_sinkhorn{_divergences}
  • revert the part of segment_sinkhorn #114 that always requires max_measure_size/num_segment in case of the 2nd interface (passing num_per_segment)
  • ad test whether barycenter problems can be jitted inside a function
  • add tests for {F}GW barycenter; fix kl GW loss not working
  • clean continuous barycenter impl.
  • polish some docstrings
  • fix syntax highlighting in some notebooks
  • fix colab badge not rendering in the docs

to be merged after #114 (because of the API change)
closes #100

@michalk8 michalk8 self-assigned this Jul 25, 2022
@michalk8 michalk8 mentioned this pull request Jul 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2022

Codecov Report

Merging #116 (fc52cfd) into main (57ef743) will decrease coverage by 2.11%.
The diff coverage is 77.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   84.38%   82.27%   -2.12%     
==========================================
  Files          43       43              
  Lines        3933     4023      +90     
  Branches      448      447       -1     
==========================================
- Hits         3319     3310       -9     
- Misses        511      588      +77     
- Partials      103      125      +22     
Impacted Files Coverage Δ
ott/core/quad_problems.py 86.97% <0.00%> (-2.80%) ⬇️
ott/tools/segment_sinkhorn.py 43.75% <0.00%> (-56.25%) ⬇️
ott/tools/sinkhorn_divergence.py 71.42% <50.00%> (-28.58%) ⬇️
ott/core/gw_barycenter.py 73.78% <60.00%> (+49.00%) ⬆️
ott/core/bar_problems.py 74.57% <67.34%> (+24.57%) ⬆️
ott/geometry/pointcloud.py 70.24% <67.74%> (-13.42%) ⬇️
ott/core/segment.py 75.00% <92.30%> (+0.45%) ⬆️
ott/geometry/low_rank.py 96.12% <94.59%> (-0.37%) ⬇️
ott/geometry/geometry.py 87.97% <96.05%> (+2.31%) ⬆️
ott/core/continuous_barycenter.py 93.75% <100.00%> (ø)
... and 25 more

@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 marked this pull request as ready for review August 2, 2022 16:34
@michalk8 michalk8 added the enhancement New feature or request label Aug 2, 2022
@michalk8 michalk8 merged commit 22d3929 into ott-jax:main Aug 3, 2022
@michalk8 michalk8 deleted the fix/padded-scaling branch August 3, 2022 12:30
michalk8 added a commit that referenced this pull request Jun 27, 2024
* [ci skip] Fix duplicate bibtex entry and a typo

* [ci skip] Update segment docs

* Add subset ixs, fix `Geometry` scaling

* Update `inv_scale_cost` in PC, privatize methods

* Update PC subsetting, LRC `inv_scale_cost`

* Rename variable, fix LRC subsetting, typing

* Add segmented Sinkhorn divergences to docs

* Minor doc fixes

* Return mask in `segment`, update Sink div

* Fix passing masks in vmap

* Fix LRC unflattening

* Fix prepare divergences in `PointCloud`

* Add masked summary test

* Refactor fixture

* Fix linter and link to PC in README.md

* Fix bug in `test_euclidean_momentum_params`

* Put epsilon-related args to `aux_data`

* Add shape test

* Test inverse scaling of masked geometry

* Add test for permutation

* Refactor scaling to use masks instead of subsets

* Fix `Geometry.inv_scale_cost`, allow `int`

* Add tests, fix some TODOs

* Fix test, do not fail fast on CI

* Update docstrings

* Add test for mask conversion

* Allow for mask in seg. Sink/Sink divergences

* Fix tree flattening

* First stab at fixing cont. barycenters

* Remove passing segmented arrays to bar problem

Also polish the documentation

* Update FGW barycenters

* Reintroduce the option of pre-segmented measures

* Update segment_point_cloud

* Fix typo in the docstring, start FGW bary tests

* Better division impl.

* Clean `GWBarycenter` impl., enable masks

* Fix segment sinkhorn, divergences

* Add GW barycenter test, fix KL loss

* Add FGW barycenter test, clean impl.

* Fix unused variables in tests, lexer and badge

* Add shape assertions, update docs
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.

handling padding when computing scaling for cost matrices.
2 participants