[MRG] (f)gw barycenter solvers new features and cleaning by cedricvincentcuaz · Pull Request #578 · PythonOT/POT
Conversation
Types of changes
-
add a new option to (un)regularized (f)gw barycenter solvers :
conv_criteriontaking values into{"barycenter", "loss"}so the user can choose convergence criterions either based on the absolute norm variations of the barycenter, or relative loss variations, respectively : The last criterion is most likely meaningful for most applications, plus suit to the ones used in inner (f)gw solvers based on conditional gradient. This feature is also added to the entropic (f)gw barycenter solvers and would come with an overhead of computing the loss at the last iteration of the inner entropic solvers. Whereas for CG inner functions, computing the barycenter loss is performed in O(#samples). -
Trying to harmonise the behavior across all (f)gw barycenter solvers :
- Some solvers were updating their convergence criterions at only few epochs to reduce the computational burden of computing norms over the barycenter, but it is really not convenient to analyse the convergence of these solvers.
- Add
fixed_structureandfixed_featurestoentropic_fused_gromov_barycenter, same than unregularized solver.
-
Adapt and improve these functions documentation.
Motivation and context / Related issue
How has this been tested (if it applies)
- Previous tests are running. Additional tests to do.
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.
Codecov Report
Merging #578 (a35211b) into master (31455e5) will increase coverage by
0.03%.
The diff coverage is100.00%.
Additional details and impacted files
@@ Coverage Diff @@ ## master #578 +/- ## ========================================== + Coverage 96.61% 96.64% +0.03% ========================================== Files 74 74 Lines 15037 15187 +150 ========================================== + Hits 14528 14678 +150 Misses 509 509
| if conv_criterion == 'barycenter': | ||
| log_['err_feature'] = [] | ||
| log_['err_structure'] = [] | ||
| log_['Ts_iter'] = [] |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am really not convinced that Ts_iter is useful to anyone, knowing that it uses a lot of memory relatively ^^'
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice PR. I think we shoumd call it stop_criterioninstead of conv it is mor eclear to me WDYT?
rflamary
changed the title
[WIP] (f)gw barycenter solvers new features and cleaning
[MRG] (f)gw barycenter solvers new features and cleaning
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters