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

3. [WIP] OASIS algorithm implementation #330

Open
wants to merge 146 commits into
base: master
Choose a base branch
from

Conversation

mvargas33
Copy link
Contributor

Hi!

I am currently implementing the OASIS algorithm and I open this PR to make the implementation transparent while working on it. Any discussion, question or comments is very welcomed.

This PR is under the WIP (Work In Progress) tag because as of now, I have a draft implementation of the algorithm out-of-the-package itself. It's a file in the root directory, with a test file in root as well.

Over these days I will move the algorithm to metric_learn folder to make it compatible with the current API. Same for testing.

Current testing only checks that nothing is broken, I'll make some test regarding KNN tasks to verify that the algorithm performs better at least for a handmade toy test.

This PR depends on the Bilinear PR #329 acceptance beforehand.

Comment on lines 122 to 134
def _vi_matrix(self, triplet):
"""
Computes V_i, the gradient matrix in a triplet
"""
# (pi+ - pi-)
diff = np.subtract(triplet[1], triplet[2]) # Shape (, d)
result = []

# For each scalar in first triplet, multiply by the diff of pi+ and pi-
for v in triplet[0]:
result.append(v * diff)

return np.array(result) # Shape (d, d)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is correct, just checked manually, but I've found a better way to compute it. This is equivalent to:

return np.outer(triplet[0], np.subtract(triplet[1], triplet[2]))

For d=100, and executing this function n=1.000.000 times, I get the following timings:

Current solution: 139.7970213389999 [s]
Using numpy outer: 13.34107805200074 [s]

I've changed it for this optimized approach

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 this pull request may close these issues.

2 participants