[MRG] Implementation of two news algorithms: SaGroW and PoGroW. by Hv0nnus · Pull Request #275 · PythonOT/POT · GitHub
Skip to content

[MRG] Implementation of two news algorithms: SaGroW and PoGroW.#275

Merged
rflamary merged 6 commits into
PythonOT:masterfrom
Hv0nnus:SaGroW
Sep 17, 2021
Merged

[MRG] Implementation of two news algorithms: SaGroW and PoGroW.#275
rflamary merged 6 commits into
PythonOT:masterfrom
Hv0nnus:SaGroW

Conversation

@Hv0nnus

@Hv0nnus Hv0nnus commented Sep 6, 2021

Copy link
Copy Markdown
Contributor

Add two new algorithms to solve Gromov Wasserstein: Sampled Gromov Wasserstein and Pointwise Gromov Wasserstein. Based on Sampled Gromov Wasserstein (Machine Learning Journal 2021) (https://hal.archives-ouvertes.fr/hal-03232509/document).

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Motivation and context / Related issue

How has this been tested (if it applies)

It has been tested on some GW problems, with various values of hyper-parameters.

Checklist

  • The documentation is up-to-date with the changes I made. (I change few lines in README.md, comment the code and document it and provide some examples in examples/gromov/plot_gromov.py.)
  • I have read the CONTRIBUTING document.
  • All tests passed, and additional code has been covered with new tests. (I do not have changed the other tests, the deletions are only a typo correction: "constratints" to "constraints")

@codecov

codecov Bot commented Sep 6, 2021

Copy link
Copy Markdown

@rflamary rflamary left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hell @Hv0nnus and thank you very much for this PR.

I did a code review to ensure that the integration in POT and its documentation is seamless, there are a few things to do but I'm confident we can merge soon.

Comment thread ot/gromov.py Outdated
Comment thread ot/gromov.py
Comment thread ot/gromov.py Outdated
Comment thread ot/gromov.py
Comment thread ot/gromov.py
Comment thread ot/gromov.py Outdated
Comment thread ot/gromov.py Outdated
Comment thread ot/gromov.py Outdated
Comment thread ot/gromov.py
Comment thread ot/gromov.py
@rflamary rflamary changed the title [MRG] Implementation of two news algorithms: SaGroW and PoGroW. [WIP] Implementation of two news algorithms: SaGroW and PoGroW. Sep 8, 2021
@Hv0nnus

Hv0nnus commented Sep 13, 2021

Copy link
Copy Markdown
Contributor Author

After further check, I think the bug is not due to any of the modification of this branch but rather a bug on the test function "test_mapping_transport_class" on the file test_da.py. This bug occurs randomly, as shown on the .PNG. This bug should be probably not be solve on this branch and an issue should be raised. I test my branch and the main one, both have the same bug.
Rdm_bug_on_test_da

@rflamary

Copy link
Copy Markdown
Collaborator

@Hv0nnus Hv0nnus changed the title [WIP] Implementation of two news algorithms: SaGroW and PoGroW. [MRG] Implementation of two news algorithms: SaGroW and PoGroW. Sep 17, 2021
@rflamary rflamary merged commit e0ba31c into PythonOT:master Sep 17, 2021
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.

2 participants