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

Constructor arguments don't seem to be passing on to the algorithm #4

Open
aadharna opened this issue Jul 9, 2020 · 1 comment
Open
Assignees
Labels

Comments

@aadharna
Copy link

aadharna commented Jul 9, 2020

I noticed that even when parameters are explicitly filled in in some of the dml constructors (e.g. NCA, LMNN), when I try to look at those values, I keep getting that the values are set to None and not the default values according to the documentation.

image


image

My first thought was that since many of the arguments shadowed your default arguments, that could be why they didn't show up, but the Nones persisted even when I changed those parameters. (But then I remembered that that makes no sense, so that couldn't be the issue.)


Also, side question: I know that algorithms of a nearest-neighbor variety are inherently expensive even when you're using the Euclidean R^N metric and not also trying to learn a metric, but when I tried to fit LMNN on a dataset of size: 6460 x 143 feels like it shouldn't be taking upwards of 10 hours. Is this extremely long runtime normal?


Finally, I just wanted to say that the survey paper that led me to your repo was wonderfully written.

Cheers,
Aaron

@jlsuarezdiaz
Copy link
Owner

jlsuarezdiaz commented Jul 9, 2020

Hi, thank you for your kind words and for pointing out this issue. There is indeed a problem with the arguments being showed in the algorithms. It is due to an incorrect naming of the variables in the constructors, with respect to what is specified in the Scikit-Learn API and I will fix it as soon as possible. However, the algorithms are handling the variables with the correct values specified through the constructor, so this problem does not affect the operation of the algorithms.

According to the expensiveness of the DML algorithms, some of the implementations, like the LMNN one, currently take a lot of time. This is something I've wanted to review for a long time, but until at least August I'm going to be too busy to get on with it. Perharps I will open a new issue to deal with this.

Cheers,
Juan Luis.

@jlsuarezdiaz jlsuarezdiaz self-assigned this Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants