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

[REVIEW] Remove unnecessary norm_add1 array from tSNE #2605

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

zbjornson
Copy link
Contributor

@zbjornson zbjornson commented Jul 24, 2020

Reduces memory usage by sizeof(float)*nRows, reduces global loads/stores and fixes an FP error.

@drobison00 I think we can actually fold this get_norm kernel into attractive_kernel_bh and get rid of the norms array also. AFAICT the denominator just works out to

      Y1[i] * Y1[i] + Y2[i] * Y2[i] + 1.0f   // norm_add1[i]
    + Y1[j] * Y1[j] + Y2[j] * Y2[j]   // norm[j]
    - 2.0f * (Y1[i] * Y1[j] + Y2[i] * Y2[j])   // the other terms

Then it'll be easier to deal with FP error in a single place because we can reorder intermediates, etc.

Did that in a separate branch for consideration: 4fa9e89

It's these norm values that get to be huge, and I'm seeing lots of cases where norm[i] + 1 != norm_add1[i] because of insufficient precision.

@zbjornson zbjornson requested a review from a team as a code owner July 24, 2020 18:54
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

Reduces memory usage by sizeof(float)*nRows and reduces global loads/stores.
@zbjornson zbjornson force-pushed the bug-tsne-remove-add1 branch from c7a12d3 to 8dc9450 Compare July 24, 2020 18:55
@zbjornson zbjornson changed the title Remove unnecessary norm_add1 array from tSNE [REVIEW] Remove unnecessary norm_add1 array from tSNE Jul 24, 2020
@zbjornson zbjornson mentioned this pull request Jul 24, 2020
@JohnZed
Copy link
Contributor

JohnZed commented Jul 24, 2020

Add to whitelist

@cjnolet cjnolet merged commit 8eb2599 into rapidsai:branch-0.15 Jul 24, 2020
@zbjornson zbjornson deleted the bug-tsne-remove-add1 branch July 24, 2020 23:35
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.

5 participants