tensorflow / tensorflow/tensorboard

Internal k-nearest-neighbors data management is broken

Open
#2,081 1 comment 0 reactions 1 assignee View on GitHub

@cannoneyed is already working on this.

Since Apr 2, 2019.

plugin:projector type:bug
Dominant language
TypeScript
Stars
7.2k
Forks
1.7k
Avg merge
4d 22h
Merged PRs (30d)
1

Description

Missed this in my review of #1901. The state management performed by
clients of computeKnn is incorrect. The intent was that computeKnn
should manage the this.nearest field entirely. Assignments on lines
361 and 384, and dereferences on 363, 387, and 388, are wrong:

https://github.com/tensorflow/tensorboard/blob/9270699784657b69a21f022dce98c439f7602d86/tensorboard/plugins/projector/vz_projector/data.ts#L358-L365

https://github.com/tensorflow/tensorboard/blob/9270699784657b69a21f022dce98c439f7602d86/tensorboard/plugins/projector/vz_projector/data.ts#L384-L388

The correct invocations should simply be:

    const knnComputation = this.computeKnn(sampledData, k)

    knnComputation.then(nearest => {
      util.runAsyncTask('Initializing T-SNE...', () => {
            this.tsne.initDataDist(nearest);
          }).then(step);
    });
    const nearest = await this.computeKnn(sampledData, nNeighbors);
    
    const nEpochs = await util.runAsyncTask('Initializing UMAP...', () => {
      const knnIndices = nearest.map(row => row.map(entry => entry.index));
      const knnDistances = nearest.map(row => 

This probably doesn’t have any observable effects at this time, because
the assignments should all be trivial and the reads should always be
reading something up-to-date, but this is still an error and should be
fixed.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.