-
Notifications
You must be signed in to change notification settings - Fork 16
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
Implemented Various Distance Metric #107
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 16031e4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add docs for distance metrics
- Resolve the conflicts
- Shift all the files from neighbours to metrics
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/neighbors/distance_metric/distance_metric.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few more changes
src/slowmokit/methods/metrics/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
src/slowmokit/methods/metrics/distance_metric/distance_metric.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the docs for distance metrics bruh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last change ig
int n = x.size(); | ||
for (int i = 0; i < n; i++) | ||
{ | ||
distance += std::pow(x[i] - y[i], power); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use std::abs(x[i] - y[i])
instead.
Otherwise there might be a problem in case of odd values of p.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
## Manhattan Distance | ||
Manhattan Distance is the sum of absolute differences between points across all the dimensions. | ||
|
||
## Manhattan Distance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change distance name to minkowski
Minkowski Distance is the generalized form of Euclidean and Manhattan Distance. | ||
|
||
## Cosine Similarity | ||
Cosine similarity is a metric, helpful in determining, how similar the data objects are irrespective of their size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add sentence like
lesser is the cosine similarity, more similar are two points.
for all four metric.
#include "distance_metric.hpp" | ||
|
||
template<class T> | ||
DistanceMetric<T>::DistanceMetric(std::vector<T> &x, std::vector<T> &y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DistanceMetric<T>::DistanceMetric(std::vector<T> &x, std::vector<T> &y) | |
DistanceMetric<T>::DistanceMetric(const std::vector<T> &x, const std::vector<T> &y) |
We don't want user to feel insecure about their data. Use const
. (No need to update in .hpp
file)
const in cpp but not in hpp?
.hpp
file
void f(int);
.cpp
file
void f(const int param1) {
return;
}
is valid and recommended.
DistanceMetric<T>::DistanceMetric(std::vector<T> &x, std::vector<T> &y) | ||
{ | ||
this->x = x; | ||
this->y = y; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DistanceMetric<T>::DistanceMetric(std::vector<T> &x, std::vector<T> &y) | |
{ | |
this->x = x; | |
this->y = y; | |
DistanceMetric<T>::DistanceMetric(std::vector<T> &x, std::vector<T> &y) : x(x), y(y) | |
{ |
Use initialiser list, they are faster.
* @param x | ||
* @param y |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add short statement describing x
and y
|
||
/** | ||
* @param power The order of the norm | ||
* @returns minkowski distance between the two vectors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @returns minkowski distance between the two vectors | |
* @returns minkowski distance between the two vectors | |
* @throws ... |
power < 1
is not allowed..
|
||
template<class T> double DistanceMetric<T>::magnitude(std::vector<T> &x) | ||
{ | ||
double result = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double result = 0; | |
long double result = 0; |
double
might overflow
|
||
template<class T> double DistanceMetric<T>::cosineSimilarity() | ||
{ | ||
double dotProduct = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double dotProduct = 0; | |
long double dotProduct = 0; |
|
||
template<class T> double DistanceMetric<T>::euclidean() | ||
{ | ||
double distance = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double distance = 0; | |
long double distance = 0; |
You will have to downcast long double
to double
finally. use std::dynamic_cast
UPDATE: Do this wherever I have added double -> long double
comments
src/slowmokit/methods/metrics/distance_metric/distance_metric.hpp
Outdated
Show resolved
Hide resolved
Fix conflicts as well. @Modernbeast02 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit names should be relevant from next time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it would be better to make all functions static instead of passing the vectors as property and then calling respective functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert CMakeLists.txt and don't change it in future models
Resolved Issue #95