Move Python scripts for building examples into tfjs-examples

[davidsoergel]

• The Python code is generally unchanged from the tfjs-layers version, but is now organized into ${example}/python
• Build scripts are adapted to a pattern where each example has ${example}/build.sh, and writes pretrained models and artifacts into ${example}/dist/data
• Build scripts now start an http server. The point of this is to serve the aforementioned artifacts, but in fact the scripts currently grab the hosted artifacts anyway, not the freshly built local ones.

These issues are outstanding but I think should be addressed in a later PR:

• TODO(soergel): Update deploy.sh to build and serve the pretrained models from their natural location under the tfjs-examples GCS bucket, not the current location under tfjs-models.
• TODO(soergel): For each example, add a UI selector to choose a local vs. hosted pretrained model

---

This change is 

[davidsoergel]

TODO on this PR: update all licenses MIT -> Apache

[nsthorat]

Review status: 0 of 27 files reviewed at latest revision, all discussions resolved.

---

mnist-transfer-cnn/build.sh, line 65 at r1 (raw file):

echo
echo "------------------------------------------------------------------"
echo "Once the HTTP server has started, you can view the demo at:"

high level question: each of these already has a yarn build and a yarn watch script - I'm wondering if you can take advantage of yarn watch instead of having 2 ways to start the example (which could get very confusing). And possibly name "build.sh" files something indicating that it's generating the static resources for the demo (maybe "build-resources" or something).

Can we separate into two distinct phases: 1) data generation in python 2) loading in JS.

We have all the build artifacts from 1) already hosted somewhere, we just need a way to inform step 2) that we're pointing to something local. This way, watching the demo is always using parcel watch (and not http-server).

Does this make sense?

---

Comments from Reviewable

[davidsoergel]

Review status: 0 of 27 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

mnist-transfer-cnn/build.sh, line 65 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

high level question: each of these already has a yarn build and a yarn watch script - I'm wondering if you can take advantage of yarn watch instead of having 2 ways to start the example (which could get very confusing). And possibly name "build.sh" files something indicating that it's generating the static resources for the demo (maybe "build-resources" or something).

Can we separate into two distinct phases: 1) data generation in python 2) loading in JS.

We have all the build artifacts from 1) already hosted somewhere, we just need a way to inform step 2) that we're pointing to something local. This way, watching the demo is always using parcel watch (and not http-server).

Does this make sense?

You are right! http-server was a vestige from the previous approach, and standardizing on Parcel would be much nicer. I made all the associated changes in my working copy and it's almost OK, except that Parcel is very bad at serving the resources.

Some perplexing serving bugs were resolved by bumping Parcel to 1.7.0, but a fatal issue remains: it refuses to serve asset files that don't have a filename extension (!). This is apparently intentional (https://github.com/parcel-bundler/parcel/blob/master/src/Server.js#L50) but I can't imagine why (or rather, the reasons that I can imagine are bad reasons). Thus, when we want to load a locally trained model, we get model.json just fine but then fetches for weights files like 'group1-shard1of1' fail because those filenames have no extension.

This is insane and seems very hard to work around. I'll think about it more in the morning.

---

Comments from Reviewable

[davidsoergel]

Review status: 0 of 25 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

mnist-transfer-cnn/build.sh, line 65 at r1 (raw file):

Previously, davidsoergel (David Soergel) wrote…

You are right! http-server was a vestige from the previous approach, and standardizing on Parcel would be much nicer. I made all the associated changes in my working copy and it's almost OK, except that Parcel is very bad at serving the resources.

Some perplexing serving bugs were resolved by bumping Parcel to 1.7.0, but a fatal issue remains: it refuses to serve asset files that don't have a filename extension (!). This is apparently intentional (https://github.com/parcel-bundler/parcel/blob/master/src/Server.js#L50) but I can't imagine why (or rather, the reasons that I can imagine are bad reasons). Thus, when we want to load a locally trained model, we get model.json just fine but then fetches for weights files like 'group1-shard1of1' fail because those filenames have no extension.

This is insane and seems very hard to work around. I'll think about it more in the morning.

OK: after discussing offline I implemented a solution that serves the HTML and JS with Parcel as usual, and serves the pretrained model files with http-server on a different port. This is transparent to the user; they can just say 'yarn watch'.

So far this approach is only used on the iris example here, pending team sanity check. If folks like this I'll propagate it to the rest.

---

Comments from Reviewable

[dsmilkov]

---

Reviewed 1 of 31 files at r1, 3 of 31 files at r2.
Review status: 4 of 25 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

iris/index.js, line 43 at r2 (raw file):

    status('Done loading pretrained model.');
  } catch (err) {
    console.log(err);

console.error(err) might be more appropriate

---

Comments from Reviewable

[nsthorat]

This approach LGTM given parcel constraints.

---

Review status: 4 of 25 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

iris/index.js, line 31 at r2 (raw file):

 */
async function loadHostedPretrainedModel(local) {
  let HOSTED_MODEL_JSON_URL;

you can do this with a ternary

const hostedModelUrlJson = local ? 'http://localhost:1235/resources/model.json' : 'https://storage.googleapis.com/tfjs-models/tfjs/iris_v1/model.json';

---

iris/index.js, line 43 at r2 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

console.error(err) might be more appropriate

when we host these on GCP, you might want to give an error in the UI that tells you using the local version doesn't make sense on external hosting, or something along those lines.

---

Comments from Reviewable

[caisq]

Review status: 4 of 25 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

iris/index.js, line 30 at r2 (raw file):

(local

Add an Args section to the doc string above to explain this arg.

---

iris/index.js, line 33 at r2 (raw file):

1235

This will unfortunately deprive this demo of the ability to use arbitrary port. But I guess it's hard to avoid this.

---

iris/build-resources.sh, line 59 at r2 (raw file):

"You can now run the demo with 'yarn watch'."

Why can't this shell script just call yarn watch for the user?

---

Comments from Reviewable

[nsthorat]

Review status: 4 of 25 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

mnist-transfer-cnn/build.sh, line 65 at r1 (raw file):

Previously, davidsoergel (David Soergel) wrote…

OK: after discussing offline I implemented a solution that serves the HTML and JS with Parcel as usual, and serves the pretrained model files with http-server on a different port. This is transparent to the user; they can just say 'yarn watch'.

So far this approach is only used on the iris example here, pending team sanity check. If folks like this I'll propagate it to the rest.

This approach LGTM.

---

iris/build-resources.sh, line 59 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

"You can now run the demo with 'yarn watch'."

Why can't this shell script just call yarn watch for the user?

I'd rather keep it in two phases. These python scripts should generate static files that get ingested by our static server (that decoupling makes the static server flow the same as when we fetch from GCP).

---

Comments from Reviewable

[davidsoergel]

Review status: 2 of 47 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

iris/index.js, line 30 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

(local

Add an Args section to the doc string above to explain this arg.

Refactored.

---

iris/index.js, line 31 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

you can do this with a ternary

const hostedModelUrlJson = local ? 'http://localhost:1235/resources/model.json' : 'https://storage.googleapis.com/tfjs-models/tfjs/iris_v1/model.json';

Done.

---

iris/index.js, line 33 at r2 (raw file):

Previously, caisq (Shanqing Cai) wrote…

1235

This will unfortunately deprive this demo of the ability to use arbitrary port. But I guess it's hard to avoid this.

Yep, this is unfortunate, but also I hope this whole approach is a stopgap until parcel-bundler/parcel#1098 is fixed.

---

iris/index.js, line 43 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

when we host these on GCP, you might want to give an error in the UI that tells you using the local version doesn't make sense on external hosting, or something along those lines.

console.error is done throughout.

The UI now tests which pretrained models are available (hosted or local) and shows only the appropriate buttons.

---

mnist-transfer-cnn/build.sh, line 65 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

This approach LGTM.

Propagated to all demos.

---

iris/build-resources.sh, line 59 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

I'd rather keep it in two phases. These python scripts should generate static files that get ingested by our static server (that decoupling makes the static server flow the same as when we fetch from GCP).

It could, but per Nikhil's previous comment it might be confusing that this would create two different ways to start the server. This way it's clear to the user that they should build the resources once, but then can start the server repeatedly. Conversely, they can start the server in one terminal, and then repeatedly rebuild the resources in another.

---

Comments from Reviewable