Skip to content

[MRG] Improve gradient computation in NCA and MLKR #113

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 17, 2018
Merged

[MRG] Improve gradient computation in NCA and MLKR #113

merged 2 commits into from
Aug 17, 2018

Conversation

wdevazelhes
Copy link
Member

Uses np.linalg.multi_dot in gradient computations in NCA and MLKR. This should provide optimal performance, no matter the values of n_components, n_features, and n_samples.

@wdevazelhes
Copy link
Member Author

I just realized that in fact in NCA one of the reason why we replaced A.dot(X.T) by X_embedded.T was that we already had it computed when computing the cost, so I'll update the PR to reuse it in NCA as it was before, and update MLKR in the same way

@wdevazelhes wdevazelhes changed the title [MRG] Use multidot in NCA and MLKR [WIP] Use multidot in NCA and MLKR Aug 17, 2018
@wdevazelhes
Copy link
Member Author

Turns out in this case where we reuse X_embedded there is no need anymore to use np.linalg.mutli_dot since the optimal order of the products is to do first X_embedded.T.dot(W_sym) and then doing this dot X. (it makes a cost of n_components* n_samples**2 + n_components * n_samples * n_features instead of n_features* n_samples **2 + n_components * n_samples * n_features)

I think the PR is ready to merge if travis passes

@wdevazelhes wdevazelhes changed the title [WIP] Use multidot in NCA and MLKR [MRG] Improve gradient computation in NCA and MLKR Aug 17, 2018
@bellet bellet merged commit acca567 into scikit-learn-contrib:master Aug 17, 2018
@wdevazelhes wdevazelhes deleted the feat/multidot_nca_mlkr branch August 22, 2018 06:48
@wdevazelhes wdevazelhes mentioned this pull request Feb 26, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bellet bellet bellet approved these changes

@perimosocordiae perimosocordiae Awaiting requested review from perimosocordiae

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wdevazelhes @bellet