This repository has been archived by the owner on Sep 16, 2021. It is now read-only.
Update to 1.1.0 and change test test setup to match new API of core. #176
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nsthorat
pushed a commit
to tensorflow/tfjs-core
that referenced
this pull request
Apr 4, 2019
MISC This PR is a step towards modularization of the library so webgl and cpu backends can live in a directory that can just be dropped entirely. The goal with this PR is to break no existing API (including things like `tf.ENV.set`, `tf.ENV.global`). Unfortunately I can't really break this change up. High-level changes: - `ENGINE` is now the global singleton root object. This is the object that is now stuck on `window` or `process` in Node (it used to be `ENV`). - `ENGINE` owns all backend registration. - `ENV` (instance of `Environment`) lives as an instance on `ENGINE` so it is also a global singleton. Its sole purpose is flags and holding `global` (`window` in browser or `process` in Node). - Environment flags are now set up with a registry on `ENV`, which allows `Environment` to have 0 dependencies. This means code that evaluates an environment flag will not pull in the world. - Unit tests now rely on the normal library backend registration (instead of having custom registries for unit tests). - When a page loads the flow is: Engine creation, Environment creation, setting flags from the URL, Flag registration, Backend registration, and lazily backend initialization when the backend is asked for. New API: - `tf.backend()` gets the current backend instance. Not to be confused with `tf.getBackend()` which returns the current backend string (don't want to break this API). Removed flags: - `BACKEND`: Flags are cached which makes this flag invalid if you switch backends. You can get the current backend name with `tf.getBackend()`. - `EPSILON`: The value of this depends on the current backend. Since backends can switch, this flag can get stale. To get epsilon you should use `tf.backend().epsilon()`. - `TEST_EPSILON`: Similar reason as above. Now you can simply use `testEpsilon()`. Detailed changes: - When registering a backend the backend is not instantiated immediately. It is instantiated lazily when something needs the current backend or the user calls `tf.backend()`. This is because constructors of backends need environment flag evaluations, which need registration to happen so we must delay the initialization of the backend. This also means that we don't initialize possibly expensive backends that don't actually get used. - `ENGINE` never gets re-instantiated during unit tests, it just gets reset (which resets all the internal state). Between unit tests we never clear registries, just instantiation of those registries (for both flags as well as engine state). - `window.ENV` => `window._tfengine` as the global so we don't possibly collide with other libraries. - `backend_webgl` now lives in `webgl/` and `backend_cpu` lives in `cpu/`. The goal will be to drop those directories if we want to have a modular build. - Constraints of unit tests now take an explicit field for backend. Previously we relied on flags but we're removing anything backend related from flags. `CPU_ENVS` and `WEBGL_ENVS` tests were testing backend-specific things, and these tests will move into the respective `webgl/` and `cpu/` folders eventually. - Optimizer cleanup, removed scalars (this was because I removed 'EPSILON' and decided to just clean them up while there). - Remove safe mode. Nobody uses it. Linked against all packages, some changes required in other repos: tensorflow/tfjs-layers#512 tensorflow/tfjs-data#176 tensorflow/tfjs-node#238 Here is a union bundle with all changes from core as well as changes above: https://storage.googleapis.com/dljs-test/env-bundle/tf1.js And a codepen showing basic usage works with this bundle: https://codepen.io/anon/pen/bJbyPx Number of tests changes slightly because I'm moving tests around and deleting some. Verified all other packages don't have a change in number of tests that were run. Number of tests locally before (`yarn test`): 9639 of 9657 Number of tests locally after (`yarn test`): 9618 of 9636 More tests in node because some tests are moved to a regular describe where they are backend-agnostic. Number of tests locally before (`yarn test-node`): 3209 Number of tests locally after (`yarn test-node`): 3261
nsthorat
changed the title
davidsoergel
approved these changes
Apr 16, 2019
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1.
Reviewable status:complete! 1 of 1 approvals obtained (waiting on @davidsoergel and @kangyizhang)
kangyizhang
approved these changes
Apr 16, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This also only registers a single webgl backend so the tests should be faster.
This change is