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

Return actual distances from MASS instead of squared distances #80

Open
rmapap opened this issue Sep 13, 2019 · 2 comments
Open

Return actual distances from MASS instead of squared distances #80

rmapap opened this issue Sep 13, 2019 · 2 comments

Comments

@rmapap
Copy link

rmapap commented Sep 13, 2019

In mass() and massStomp() in utils.py, the quotient in the calculation of the squared distance can go slightly above 1, leading to a negative difference (see line 177 and line 200).

Is there any objection to just wrapping the calculation of the squared distance in np.clip(., 0.0, None) and then taking the square root of that?

This issue is addressed in distanceProfile.py by allowing complex values: line 66, line 118, line 126. scrimp.py seems to implement its own version of MASS, and takes the absolute value before taking the square root (see #63): line 71, line 162, line 206, line 257.

SCRIMP++ aside, it seems like it might be cleaner to just clip negative values to 0 directly in mass() and massStomp().

@tylerwmarrs
Copy link
Contributor

@rmapap In regards to SCRIMP++ using absolute value - the Matlab implementation (from the UCR group) also takes this approach in resolving the issue you mention. Please refer to the code on their SCRIMP++ resource page: https://sites.google.com/site/scrimpplusplus/

@rmapap
Copy link
Author

rmapap commented Sep 13, 2019

@tylerwmarrs Thanks for the quick reply. I think that when this issue occurs, the values are essentially zero (e.g., -1e-12), so whether you set the value to 0 or make it positive probably doesn't really matter (except that np.abs() is probably faster). My main concern was just with having to wrap every call to MASS in np.sqrt()

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

No branches or pull requests

2 participants