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

[MRG] Correct pointer overflow in EMD #381

Merged
merged 10 commits into from
Jun 13, 2022

Conversation

ncassereau
Copy link
Contributor

@ncassereau ncassereau commented Jun 3, 2022

Motivation and context / Related issue

When doing a transport plan with EMD between two 50k points clouds with uniform probability, the transport matrix would be a sparse matrix with only 42950 non-zero values. When solving the assignment problem between two pictures (as in https://github.com/ncassereau-idris/pictures-optimal-transport), this meant that some points would evade to the origin rather than their respective target points. This is unexpected, as the correct transport matrix should be a sparse matrix with 50k nonzero values.

What happened then ?

When computing transport plan with EMD, some pointers remained as int. This meant that if your cost matrix is over 46k lines and columns, pointers would overflow and the cost matrix would not be correctly converted.

How has this been tested (if it applies)

After modifying the openmp version, I tried to make a transport between two clouds of 50k points, and successfully obtained a sparse matrix with 50k nonzero values. I will try to use it on the picture assignment problem as well as execute unit tests to make sure the given matrix makes sense.

A picture assignment problem was solved with 60k points and produced a sound gif, therefore the fix is working correctly. At 70k and above, there is a risk of infeasible problem.

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.

@ncassereau ncassereau marked this pull request as ready for review June 8, 2022 14:38
@ncassereau ncassereau changed the title [WIP] Correct pointer overflow in EMD [MRG] Correct pointer overflow in EMD Jun 8, 2022
@codecov
Copy link

codecov bot commented Jun 8, 2022

Codecov Report

Merging #381 (ee08444) into master (1f30759) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
+ Coverage   93.76%   93.79%   +0.03%     
==========================================
  Files          23       23              
  Lines        5934     5934              
==========================================
+ Hits         5564     5566       +2     
+ Misses        370      368       -2     

@rflamary rflamary merged commit e547fe3 into PythonOT:master Jun 13, 2022
@ncassereau ncassereau deleted the emd_high_dim branch June 13, 2022 12:53
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