sinkhorn_barycenter refactor

[zsteve]

Hi all,

I've done a quick refactor of sinkhorn_barycenter to now conform to conventions introduced in #100 (which I'm sorry I missed at the time due to moving), reusing code from sinkhorn.jl and sinkhorn_gibbs.jl where possible.

Problem parameters are specified in SinkhornBarycenterSolver, and the standard scaling algorithm corresponds to setting alg = SinkhornBarycenterGibbs()

I've had to define SinkhornBarycenterGibbsCache separately because in order to do the convergence checks, I need to keep track of the barycenter at each iteration, named a in the code. For the convergence check I use the existing SinkhornConvergenceCache struct.

Let me know if there's anything further that can be cleaned up in the code.

One thing I will note here is that quite a few parts of solve! can be reused between SinkhornGibbs and SinkhornBarycenterGibbs -- essentially the differences come down to the update steps and the arguments passed to the convergence check. One solution here could be to wrap the update steps into algorithm-specific calls via multiple dispatch. I haven't attempted this yet (maybe a future PR?) but I'd be keen to hear your thoughts.

[devmotion]

Sorry for not replying earlier, I just came back from vacation 🙂

One thing I will note here is that quite a few parts of solve! can be reused between SinkhornGibbs and SinkhornBarycenterGibbs -- essentially the differences come down to the update steps and the arguments passed to the convergence check. One solution here could be to wrap the update steps into algorithm-specific calls via multiple dispatch. I haven't attempted this yet (maybe a future PR?) but I'd be keen to hear your thoughts.

If this is possible, it could be a good way to reduce code and ensure that the implementations are consistent. However, I think this can be left for a future PR.

[coveralls]

Pull Request Test Coverage Report for Build 1161321360

• 69 of 70 (98.57%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.5%) to 96.711%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/entropic/sinkhorn_barycenter.jl	49	50	98.0%

Totals
Change from base Build 1158342145:	0.5%
Covered Lines:	588
Relevant Lines:	608

---

💛 - Coveralls

[codecov-commenter]

Codecov Report

Merging #110 (6e14eda) into master (5354fb6) will increase coverage by 0.45%.
The diff coverage is 98.57%.

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
+ Coverage   96.25%   96.71%   +0.45%     
==========================================
  Files          11       12       +1     
  Lines         561      608      +47     
==========================================
+ Hits          540      588      +48     
+ Misses         21       20       -1     

Impacted Files	Coverage Δ
src/entropic/sinkhorn_barycenter.jl	98.00% <98.00%> (+6.69%)	⬆️
src/entropic/sinkhorn_barycenter_gibbs.jl	100.00% <100.00%> (ø)
src/utils.jl	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5354fb6...6e14eda. Read the comment docs.