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

refactoring UnivariateSolver #472

Merged
merged 4 commits into from
Nov 27, 2023
Merged

refactoring UnivariateSolver #472

merged 4 commits into from
Nov 27, 2023

Conversation

marcocuturi
Copy link
Contributor

@marcocuturi marcocuturi commented Nov 24, 2023

Follow up on @Daniel-Packer 's implementation

  • New API for UnivariateSolver more in agreement with other implementations (takes a problem as input now)
    • In particular, UnivariateSolver can now take a standard prob on point clouds of size [n,d]. This means that d independent slices will be considered, running 1D OT on each of them.
  • Implementation for fractional costs a and b of all flavors, and simplifications (essentially no need to pass method string, only arguments such as num_subsamples or quantiles. If those are passed then user intends to use these alternative methods. Otherwise, select automatically the good method by checking problem's nature (in particular n=m and weights were uniform (only weaknesses: only checks if those were by default, don't see obvious workaround to test numerically)
  • No option to use soft sorting operators. Not completely sure this makes so much sense as of now, so I simplified
  • Also Output OT matrix, even for fractional cost, in a new UnivariateOutput namedtuple.
  • UnivariateWasserstein is now a new cost for point clouds. This makes sense for lower_bound.py and slightly alleviates code there, rather than using the double vmap that is considered by default in the .pairwise method of a CostFn

Copy link

codecov bot commented Nov 24, 2023

Codecov Report

Merging #472 (6a77a9b) into main (75cdd11) will decrease coverage by 0.04%.
The diff coverage is 86.66%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #472      +/-   ##
==========================================
- Coverage   90.45%   90.41%   -0.04%     
==========================================
  Files          58       59       +1     
  Lines        6350     6440      +90     
  Branches      894      912      +18     
==========================================
+ Hits         5744     5823      +79     
- Misses        467      472       +5     
- Partials      139      145       +6     
Files Coverage Δ
src/ott/geometry/distrib_costs.py 100.00% <100.00%> (ø)
src/ott/math/utils.py 95.00% <100.00%> (+0.45%) ⬆️
src/ott/problems/linear/linear_problem.py 100.00% <100.00%> (ø)
src/ott/tools/sinkhorn_divergence.py 91.04% <100.00%> (ø)
src/ott/solvers/quadratic/lower_bound.py 75.00% <46.15%> (-5.77%) ⬇️
src/ott/solvers/linear/univariate.py 88.07% <87.00%> (-6.48%) ⬇️

... and 1 file with indirect coverage changes

@marcocuturi
Copy link
Contributor Author

also of interest for @JTT94 since could impact soft sorting initializers

src/ott/geometry/distrib_cost.py Outdated Show resolved Hide resolved
src/ott/geometry/distrib_cost.py Outdated Show resolved Hide resolved
src/ott/geometry/distrib_cost.py Outdated Show resolved Hide resolved
src/ott/geometry/distrib_cost.py Outdated Show resolved Hide resolved
src/ott/geometry/distrib_cost.py Outdated Show resolved Hide resolved
src/ott/solvers/quadratic/lower_bound.py Outdated Show resolved Hide resolved
src/ott/solvers/linear/univariate.py Outdated Show resolved Hide resolved
src/ott/solvers/quadratic/lower_bound.py Show resolved Hide resolved
src/ott/solvers/quadratic/lower_bound.py Outdated Show resolved Hide resolved
src/ott/geometry/distrib_cost.py Outdated Show resolved Hide resolved
@marcocuturi marcocuturi merged commit fb7b76a into main Nov 27, 2023
9 of 11 checks passed
@marcocuturi marcocuturi deleted the univ_refactor branch November 27, 2023 21:50
michalk8 pushed a commit that referenced this pull request Jun 27, 2024
Merging as docs issue comes from recent `neural` refactoring.

* refactoring

* refactoring following michal comments

* fix tests

* last fixes
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.

None yet

2 participants