-
Notifications
You must be signed in to change notification settings - Fork 506
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
Infeasible solution for EMD but inputs are in the simplex? #126
Comments
By the way, looks related to #93 which does not have a resolution. I also am able to reproduce this on both Linux and Mac OS. |
Hello @alsuhr-c , thank you for the bug report I believe the problem comes from the fact that on the simplex in float32 is sometimes not on the simplex in float64 (up to numerical precision). In other words if you do
on the numpy float64 vectors it should work. Could you print the float for the sum with 16 digits '%1.16f') so that we can confirm the error please? I have seen the same probem in my code when going from pytorch to numpy to solve emd. I think we should provide also pytorch compatible functioin that can handle this kind if problems in POT but i never find the time to do a clean PR . |
I inserted the lines:
between the softmaxes and printing the distributions, and printed using
I attached the minimal working example as a text file (Github doesn't allow py attachments I guess) if you want to try it: test_emd.txt |
One work-around I am using now instead is to set all values less than a certain amount (has to be high, though) to zero, e.g.,
This will return a feasible solution, but the threshold must be relatively high (e.g., like above, I also realized that |
@alsuhr-c I think I found the problem! The .numpy() function in pytorch return float32 numpy arrays. If i convert them manually with import ot
import torch
import random
import numpy as np
# Get distributions
random_dist = torch.softmax(torch.tensor([random.random() * 10 for _ in range(4)]), dim=0).numpy().astype(np.float64)
random_dist2 = torch.softmax(torch.tensor([random.random() * 10 for _ in range(4)]), dim=0).numpy().astype(np.float64)
random_dist /= random_dist.sum()
random_dist2 /= random_dist2.sum()
print('--- First dist ---')
print('Shape: %s' % random_dist.shape)
print('Sum: %1.64f' % np.sum(random_dist))
print('Max: %1.64f' % random_dist.max())
print('Min: %1.64f' % random_dist.min())
print('--- Second dist ---')
print('Shape: %s' % random_dist2.shape)
print('Sum: %1.64f' % np.sum(random_dist2))
print('Max: %1.64f' % random_dist2.max())
print('Min: %1.64f' % random_dist2.min())
# Create distance matrix
coords = np.reshape(np.asarray(np.meshgrid(np.linspace(0, 1, 2), np.linspace(0, 1, 2)))[[1, 0], :, :].transpose((1, 2, 0)), [-1, 2])
tiled = np.tile(coords[:, np.newaxis, :], [1, coords.shape[0], 1])
ttiled = tiled.transpose((1, 0, 2))
distances = np.array(np.linalg.norm(tiled - ttiled, axis=2), order='C')
# Compute EMD
ot.emd(random_dist, random_dist2, distances) Now it should work. We definitely have to tell people that teh conversion to float6 i happening with a warning in the documentation. You are welcome to propose a PR if you want. |
Interesting. This works for the Linux machine, but still a problem for Mac (I tested it by fixing the random seed). For me, this is not a huge deal as I run experiments on Linux. But I noticed issues saying people are running into the problem with Mac OS. I can try to create a PR. Specifically, raising a Also, what do you think about raising an error, rather than a warning, when a problem is infeasible? I'm not super familiar with what is going on behind the scenes in the EMD computation, but is it ever possible that the problem is infeasible when the inputs are on the simplex? If not, then I think raising an error would be a lot safer than raising a warning. Warnings are only shown once to the user, and the result of the call to Another option, instead of actually raising an error, is to have the return value of |
@ncourty could you have a look at the code above on Mac OS? This is indeed very weird. |
I confirm that the above code is rising the 'Problem infeasible' UserWarning without the My configuration is actually
|
Ah, never mind, I was misreading where the error was coming from on the Mac! (I was trying resetting tiny values to zero and then running EMD, and this unsurprisingly resulted in an infeasible solution). The solution of converting to float64 and then dividing by the sum works on Mac. Any thoughts on changing how the EMD solver reports this problem to the user? I think raising an exception would be safest. The user could catch such an exception if they want and ignore it, but there wouldn't be a return value that looks like a solution. |
hello @alsuhr-c , yes i definitely agree that raising an exception might is safest. If you feel like making a PR, go ahead ! Otherwise we'll try to take some time to implement it for the next release. |
I am wondering if the bug has been fixed. As @alsuhr-c suggested, throwing an exception would be better. The current implementation will just return a zero matrix for the transport plan. This is very dangerous as it looks like nothing goes wrong. |
yes it has and it will e in the next release very soon. in the meantime you can install from the master branch in git that is very stable |
Describe the bug
A call to the EMD solver is raising a
UserWarning
about the problem being infeasible (Check that a and b are in the simplex
). I dug into the code and this seems to be raised only when a value in one of the two inputs is less than zero. However, I am printing the sum, max, and min of the inputs and don't see any odd behavior (sum to one, max values not greater than 1, min values not less than zero; inputs are the same size; type isnp.float64
).Could this be an issue with underflow?
To Reproduce
Steps to reproduce the behavior: using python3.7:
Example output:
Expected behavior
I would expect that this error is not thrown as all of the values in both distributions are nonnegative.
Screenshots
N/A, minimal working example above
Desktop (please complete the following information):
Output of the following code snippet:
Linux:
Mac:
Additional context
The scale for
random.random()
can be as low as10
and the bug will happen. If the scale is 1, it can find a solution. Sounds like a numerical underflow problem to me.Thanks!
The text was updated successfully, but these errors were encountered: