Skip to content

Commit

Permalink
projector: Fix esbuild-bundled projector. (#5944)
Browse files Browse the repository at this point in the history
The change to use the esbuild bundler (#5829) broke the projector plugin (#5924).

The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the
bundler/minification code:

https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696

The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope.

We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.
  • Loading branch information
bmd3k authored Sep 22, 2022
1 parent 12a30c3 commit 6a74b34
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 2 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,10 @@ jobs:
! git grep -E '"@npm_angular_bazel//:index.bzl"' 'tensorboard/**/BUILD'
- run: |
! git grep -E 'mat-color|$mat-' 'tensorboard/**/*.scss'
# Make sure no one depends on the numeric library directly. Please
# import the indirection in //tensorboard/webapp/third_party.
- run: |
! git grep -E '@npm//numeric' 'tensorboard/*/BUILD' ':!tensorboard/webapp/third_party/**'
lint-misc: # build, protos, etc.
runs-on: ubuntu-18.04
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/plugins/projector/vz_projector/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ tf_ts_library(
"//tensorboard/components/polymer:register_style_dom_module",
"//tensorboard/components/tf_wbr_string",
"//tensorboard/webapp/third_party:d3",
"//tensorboard/webapp/third_party:numeric",
"//tensorboard/webapp/third_party:tfjs",
"@npm//@polymer/decorators",
"@npm//@polymer/polymer",
"@npm//numeric",
"@npm//three",
"@npm//umap-js",
],
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/plugins/projector/vz_projector/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import numeric from 'numeric';
import {UMAP} from 'umap-js';
import {numeric} from '../../../webapp/third_party/numeric';
import {TSNE} from './bh_tsne';
import {SpriteMetadata} from './data-provider';
import * as knn from './knn';
Expand Down
9 changes: 9 additions & 0 deletions tensorboard/webapp/third_party/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@ tf_ts_library(
"@npm//marked",
],
)

tf_ts_library(
name = "numeric",
srcs = ["numeric.ts"],
strict_checks = False,
deps = [
"@npm//numeric",
],
)
32 changes: 32 additions & 0 deletions tensorboard/webapp/third_party/numeric.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/* Copyright 2022 The TensorFlow Authors. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import numeric from 'numeric';

// The numeric library requires that the symbol 'numeric' is available in the
// global scope when its operations are executed by other modules. See, for
// example, how the definition of some of its operations refer to the string
// 'numeric' in the Function definition, unmodifiable by the
// bundler/minification code:
//
// https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696
//
// The esbuild bundler does not keep 'numeric' in global scope and instead
// renames it as part of bundling/minification. We work around this by manually
// adding it to global scope here.
window['numeric'] = window['numeric'] ?? numeric;

// Reexport the numeric library. All imports of numeric should be done through
// this file to ensure 'numeric' is available in the global scope.
export {numeric};

0 comments on commit 6a74b34

Please sign in to comment.