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

Distance calculation with additional data #8

Open
Arlodotexe opened this issue Sep 9, 2023 · 3 comments · May be fixed by #9
Open

Distance calculation with additional data #8

Arlodotexe opened this issue Sep 9, 2023 · 3 comments · May be fixed by #9

Comments

@Arlodotexe
Copy link

Arlodotexe commented Sep 9, 2023

I'm using your UMAP library in a project and have encountered a limitation that I believe could be addressed with an enhancement to the library.

In my project, I need to access data beyond the vectors themselves during the distance calculation. However, the current implementation of the DistanceCalculation delegate public delegate float DistanceCalculation(float[] x, float[] y) does not support generics, which restricts access to additional data regarding a set of vectors.

One potential solution could be to modify the DistanceCalculation function to support generics. This would allow for extended data access during the distance calculation, increasing the flexibility and applicability of the library for projects like mine that require access to additional data in order to calculate distance.

I understand that this is a non-trivial change and may not align with the current design of the library. However, I believe this enhancement could significantly improve the library's versatility.

I couldn't find another C# implementation of UMAP, and your library has been very useful in my project so far. I'm considering forking the library to implement this change myself, but I wanted to bring this to your attention first in case this is an enhancement you'd be interested in incorporating.

Thank you for your time and consideration.

@theolivenbaum
Copy link
Collaborator

Hi @Arlodotexe, that's a great suggestion, but I'm afraid it might be a bit complicated to implement due to how the calculation works. If you want to give it a try on a PR, happy to merge it if it works!

@amaid
Copy link

amaid commented Sep 19, 2023

Hi @theolivenbaum ,
I'm forking the repo in an attempt to fix the issue.

@theolivenbaum
Copy link
Collaborator

Hi @theolivenbaum ,
I'm forking the repo in an attempt to fix the issue.

Ok! If you manage go get a nice solution feel free to open a PR!

@amaid amaid linked a pull request Sep 24, 2023 that will close this issue
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 a pull request may close this issue.

3 participants