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

Move examples from ml5-example to the core ml5-library repo #831

Merged
merged 20 commits into from
Mar 31, 2020

Conversation

bomanimc
Copy link
Member

Context

This is PR proposes significant changes to the library structure and deployment process! See full context on the reasoning behind these changes in #809.

This PR focuses on preparing
The base branch is bomani.add-examples, which is a branch off of the development branch in this repository which contains a direct copy of the contents for the development branch on the ml5-examples repo. The reason I did things this way was to make it easier to see relevant changes in this PR by separating out the hundreds/thousands of files involved in porting over code from ml5-examples into this repo.

Changes

Summary of the important changes:

  • Move all of the necessary code from the ml5-examples repo into a folder named examples/ in this repo.
  • Update the scripts so that the examples can be served (using live-server) while referencing a reloading version of the ml5.js library built by a concurrently running webpack-dev-server instance
  • Update build scripts to copy the contents of the examples/ folder into a static-deployment-ready folder called dist_examples/. During this process, all references to the locally served version of the ml5.js library are automatically replaced with a reference to the @latest UNPKG version of ml5.js.
  • Update documentation, including READMEs, templates, and link references to example files in the documentation so that they point to this repo.

It's important to also note that this change, in its current form, increases the size of this repository from 793MB (on the current development branch) to 1.6GB (on this branch). Both numbers are measured after node_modules have been installed on my MacBook Pro running Mojave. These numbers are probably rough and may differ between devices, but adding them here to give a sense of the difference.

I verified that the size of the bundle served to NPM through the dist folder has not increased as a result of these changes.

Updated Workflow

You can start a server or your machine to view a local version of the ml5 examples index web page that links to every example in the project. You can use this local version of the examples index to test new and old examples against changes you're making to the core library.

Open your terminal

# change directories
cd ml5-library

# install the dependencies
npm install

# run the local web server
npm run develop

After running the above commands, a local version of the ml5 examples index web site should open at http://localhost:8081/ in your default browser. The page will automatically refresh itself each time a change is made to any file in the examples/ directory. Additionally, the npm run develop command triggers webpack-dev-server to build the ml5.js core library file and serve it at http://localhost:8080/. Since all of the examples load their copies of ml5.js from http://localhost:8080/ml5.js, you'll be able to see how core library changes influence your examples in real-time.

For example, if I change ml5-library/examples/p5js/BodyPix/BodyPix_Image/index.html, the browser will refresh automatically. If I change ml5-library/src/BodyPix/index.js the browser will also refresh (in the future, we can consider adding hot module reloading for this case).

Deploying Examples

After these changes, we'll also need to switch the deployment process, since we'll no longer be using GH Pages to deploy the examples site.

Suggest approach: Create a new Netlify project to serve the examples online (e.g. examples.ml5js.org) and trigger builds when new changes are pushed to the release branch. We should specify Netlify to run the npm run examples:build command and serve the resulting dist_examples/ folder. I'd suggest serving it at examples.ml5js.org, and I'm happy to set this up if I can be given access to the Netlify team.

Next Steps & Open Questions

Please see #809 for further discussion on the merits and drawbacks of this high-level approach. I've left recent comments there with my remaining concerns about how a change like this influence the library in the long-term.

Furthermore, we also should consider how to decrease the overall repository size. Currently, we have a variety of model files that are over 5MB:

javascript/ImageClassification/ImageClassification_Video_Load/model/image-model.weights.bin (5.62 MiB)
  p5js/ImageClassification/ImageClassification_Video_Load/model/image-model.weights.bin (5.62 MiB)
  javascript/Pix2Pix/Pix2Pix_promise/models/edges2pikachu.pict (13 MiB)
  javascript/Pix2Pix/Pix2Pix_callback/models/edges2pikachu.pict (13 MiB)
  p5js/Pix2Pix/Pix2Pix_callback/models/edges2pikachu.pict (13 MiB)
  p5js/Pix2Pix/Pix2Pix_promise/models/edges2pikachu.pict (13 MiB)
  javascript/Word2Vec/Word2Vec_Interactive/data/wordvecs5000.json (13.7 MiB)
  p5js/Word2Vec/Word2Vec_Interactive/data/wordvecs5000.json (13.7 MiB)
  javascript/Word2Vec/Word2Vec_Interactive/data/wordvecs10000.json (27.3 MiB)
  p5js/Word2Vec/Word2Vec_Interactive/data/wordvecs10000.json (27.3 MiB)
  p5js/CharRNN/CharRNN_Interactive/models/woolf/rnnlm_multi_rnn_cell_cell_0_basic_lstm_cell_kernel (32 MiB)
  javascript/CharRNN/CharRNN_Interactive/models/woolf/rnnlm_multi_rnn_cell_cell_0_basic_lstm_cell_kernel (32 MiB)
  javascript/CharRNN/CharRNN_Text_Stateful/models/woolf/rnnlm_multi_rnn_cell_cell_0_basic_lstm_cell_kernel (32 MiB)
  javascript/CharRNN/CharRNN_Interactive/models/woolf/rnnlm_multi_rnn_cell_cell_1_basic_lstm_cell_kernel (32 MiB)
  javascript/CharRNN/CharRNN_Text/models/woolf/rnnlm_multi_rnn_cell_cell_1_basic_lstm_cell_kernel (32 MiB)
  p5js/CharRNN/CharRNN_Text/models/woolf/rnnlm_multi_rnn_cell_cell_0_basic_lstm_cell_kernel (32 MiB)
  p5js/CharRNN/CharRNN_Text/models/woolf/rnnlm_multi_rnn_cell_cell_1_basic_lstm_cell_kernel (32 MiB)
  p5js/CharRNN/CharRNN_Text_Stateful/models/woolf/rnnlm_multi_rnn_cell_cell_1_basic_lstm_cell_kernel (32 MiB)
  javascript/CharRNN/CharRNN_Text/models/woolf/rnnlm_multi_rnn_cell_cell_0_basic_lstm_cell_kernel (32 MiB)
  p5js/CharRNN/CharRNN_Text_Stateful/models/woolf/rnnlm_multi_rnn_cell_cell_0_basic_lstm_cell_kernel (32 MiB)
  javascript/CharRNN/CharRNN_Text_Stateful/models/woolf/rnnlm_multi_rnn_cell_cell_1_basic_lstm_cell_kernel (32 MiB)
  p5js/CharRNN/CharRNN_Interactive/models/woolf/rnnlm_multi_rnn_cell_cell_1_basic_lstm_cell_kernel (32 MiB)

@joeyklee
Copy link
Contributor

Hi @bomanimc - Super cool what you've done here!

A few notes:

in package.json:
Instead of --mount=/:./examples/public", probably better to do --mount=/:./examples", like below. This is because the site in examples/public requires the examples.json to be updated on every chance since that is where the site at examples/public gets the examples list from. By removing the public from the mount point, the live-server will just show the examples directory for people to add their own examples to.

"examples:serve": "live-server ./examples --port=8081 --mount=/:./examples",
  • We should make sure ot run the npm run examples:update-json before we build the examples with npm run build:examples.

  • Personally I really like that that examples are right in the ml5-library. It is super nice to be able to run :npm run start and npm run examples:serve and voila, its development time! It definitely brings all of the pieces together in a unified way such that updates to the examples can match updates to the library.

My vote would be to merge this in and refine as needed! 💯

@bomanimc
Copy link
Member Author

Thanks for the comments @joeyklee! Good idea to run npm run examples:update-json before the examples build.

One question!

This is because the site in examples/public requires the examples.json to be updated on every chance since that is where the site at examples/public gets the examples list from. By removing the public from the mount point, the live-server will just show the examples directory for people to add their own examples to.

@joeyklee just to make sure I understand, would the goal of this change be to ensure that people who run npm run examples:serve are reminded to update the examples JSON file because they see the file on the served site?

@bomanimc bomanimc force-pushed the bomani.examples-monorepo-test branch from fb5c6de to 634ea88 Compare March 26, 2020 15:46
@bomanimc bomanimc force-pushed the bomani.add-examples branch from 19f960f to a660ab7 Compare March 26, 2020 15:52
@joeyklee
Copy link
Contributor

@joeyklee just to make sure I understand, would the goal of this change be to ensure that people who run npm run examples:serve are reminded to update the examples JSON file because they see the file on the served site?

If we run npm run examples:serve with the current implementation, the "fancy cool" website from the /public directory gets served up. However, the /public/index.html file in the /public directory will show the URLs contained within the examples.json. If we do not run the npm run examples:update-json before the running npm run examples:serve, then the developer will not see any new example folder they created. Does that make sense? Apologies if that explanation is confusing. I'm happy to clarify!

So the idea of not specifically serving the /public/index.html in development would be to allow the live-server to live update the examples directory.

@bomanimc
Copy link
Member Author

bomanimc commented Mar 26, 2020

So the idea of not specifically serving the /public/index.html in development would be to allow the live-server to live update the examples directory.

Ahh okay, I think I understand! So the mount argument in the current version of the command live-server ./examples --port=8081 --mount=/:./examples/public essentially links the subdirectory ./examples/public to the route /. I added this here so that the URL of each example page would be something like http://127.0.0.1:8081/javascript/BodyPix/BodyPix_Image/ rather than http://127.0.0.1:8081/public/javascript/BodyPix/BodyPix_Image/ and also so that the index.html page would be the first thing to load when the live server opens a window, which I think makes the experience slightly simpler. That said, the live-server is still watching the entire examples directory because of the live-server ./examples portion, so any changes to the examples.json trigger a browser refresh. I've tested this by editing the examples.json file to ensure that the browser refreshes and testing that removing an item from examples.json and then running npm run examples:update-json restores the missing item and automatically refreshes the browser via live-server.

That said, the development process still requires people to remember to run npm run examples:update-json script. We could potentially make changes to have the file rebuilt when changes are made to the directory (would probably need a new tool to do this) or add some UI on the examples index that only appears when the window.location.path is a localhost or 127.0.0.1 URL.

Let me know if that makes sense and/or addresses what you were mentioning!

@joeyklee
Copy link
Contributor

That said, the development process still requires people to remember to run npm run examples:update-json script. We could potentially make changes to have the file rebuilt when changes are made to the directory (would probably need a new tool to do this) or add some UI on the examples index that only appears when the window.location.path is a localhost or 127.0.0.1 URL.

  • Yep! You understood this correctly. I think either:
    A. we allow the live-server to just serve up the examples directory and the developers can navigate to the folder/feature of interest. The developer would just need to remember to rebuild the examples.json (or we could just handle this on netlify or with CI).
    B. we decide that the public/index.html as the source of truth such that any updates to the examples repo -- any creation of a new directory -- are automatically added to the examples.json and reflected in the view of public/index.html. As you've pointed out this option would take a bit more tooling and probably would mean that this would get rebuilt every time a file is saved which might not be the most efficient use of computer resources.

I can see the advantages and disadvantages of both. I would be more prone to option A for now, and keep the public/index.html as just our public facing interface for exploring examples.

@bomanimc
Copy link
Member Author

OH! okay I fully understand now! The benefit of serving the examples folder instead of the interface is that people don't have to worry about running the npm run examples:update-json script, since they can just navigate to the location of their changes directly.

I agree with you! Option A sounds like the best bet. I'll update this PR to use: live-server ./examples --port=8081 --mount=/:./examples. Thanks for helping me to understand!

@joeyklee
Copy link
Contributor

I agree with you! Option A sounds like the best bet. I'll update this PR to use: live-server ./examples --port=8081 --mount=/:./examples. Thanks for helping me to understand!

  • Of course! Thanks so much for your thorough investigation! Apologies for any confusing wording!

@bomanimc
Copy link
Member Author

Updated! I'll wait to see if anyone has feedback before moving forward here.

Taken from #809, here are some of the next steps I think we can take (in order):

I'll start with number #2 of this process!

Also listing some of the steps that we'd want to take should we decide to move forward with this plan:

On the ml5-examples repository:

  1. (Potentially) Announce plans for changes to the repository over communication channels with people who use or contribute to ml5.js.
  2. ml5-examples: Merge or close all of the open pull requests.
  3. ml5-library: Update the bomani.add-examples branch with a copy of the latest changes to the ml5-examples repo.
  4. ml5-library: Merge changes from bomani.examples-monorepo-test into bomani.add-examples.
  5. ml5-library: Merge changes from bomani.add-examples into development.
  6. ml5-library: Merge changes from this branch into the release branch This step completes the change to add examples to this repository.
  7. ml5-library: Deploy a version of the ml5 examples index based on the code in this repo.
  8. ml5-examples: Update the ml5 examples index deployed through this directory to redirect (either automatically or by making the site say "The examples now live at...")
  9. ml5-examples: Update the README to denote that the repo has been deprecated.
  10. ml5-examples: Archive the repository, preventing further changes to the codebase and signaling to viewers that the repo has been closed.
  11. ml5-examples: Transfer all of the issues to the ml5-examples repo: https://help.github.com/en/github/managing-your-work-on-github/transferring-an-issue-to-another-repository
  12. ml5-library: Since many of the issues on the ml5-examples repo were addressed by changes to ml5-library or have corresponding issues, we should go through the new issues that have been moved from ml5-examples to close/label/triage these issues.

For step 1, I'd love to get feedback on if this is necessary and the best way to communicate with the community!

Please let me know if there's anything I'm overlooking here!

@joeyklee
Copy link
Contributor

(Potentially) Announce plans for changes to the repository over communication channels with people who use or contribute to ml5.js.

  • I might actually run step 10 - archive the examples repo to flag that the repo is no longer active - sooner than later to signal the change. That would be the clearest way to indicate that the migration is happening! If this is too drastic, Then I'd add a big bold header on the ml5-examples repo stating that "this repo is being archived soon!"

@bomanimc
Copy link
Member Author

Thanks for the suggestion @joeyklee and for your comments on ml5js/ml5-examples#262 and #790!

I thought about the process a bit more, and I think it may make sense to follow your suggestion of archiving the repo sooner rather than later. Instead of trying to merge multiple PRs into the examples repo now as I suggested in my process above, it probably is safer to reopen PRs for those changes in this repository once the transition is complete. This reduces the number of changes we're working with for this examples-library merger, avoids forcing us to merge in anything we still might want to polish before releasing, and saves time fixing conflicts on the ml5-examples repo.

I think what would make sense is to:

  1. Follow your suggestions of adding a note at the top of the README on ml5-examples notifying people of the changes.
  2. Comment on the remaining PRs on ml5-examples letting the authors know to resubmit the PRs on this repo.
  3. Focus on getting the changes in this PR merged in.
  4. Archive ml5-examples

@joeyklee
Copy link
Contributor

I thought about the process a bit more, and I think it may make sense to follow your suggestion of archiving the repo sooner rather than later. Instead of trying to merge multiple PRs into the examples repo now as I suggested in my process above, it probably is safer to reopen PRs for those changes in this repository once the transition is complete. This reduces the number of changes we're working with for this examples-library merger, avoids forcing us to merge in anything we still might want to polish before releasing, and saves time fixing conflicts on the ml5-examples repo.

  • Agreed. Reducing as much of the conflict resolution madness is ideal!

I think what would make sense is to:
Follow your suggestions of adding a note at the top of the README on ml5-examples notifying people of the changes.

  • Sounds good!

Comment on the remaining PRs on ml5-examples letting the authors know to resubmit the PRs on this repo.

  • This also works for me.

Focus on getting the changes in this PR merged in.

  • Perfecto.

Archive ml5-examples

  • Boom!

Copy link
Contributor

@joeyklee joeyklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bomanimc - based on our discussions and the current roadmap I approve this PR!

@bomanimc bomanimc force-pushed the bomani.add-examples branch from a660ab7 to 78fd1e3 Compare March 29, 2020 21:03
@bomanimc bomanimc force-pushed the bomani.examples-monorepo-test branch from abd7c78 to 4202f82 Compare March 29, 2020 21:11
@bomanimc
Copy link
Member Author

@joeyklee I think that the changes in #846 should fix the test errors we're seeing on this build. That's because one of the errors here related to a batch normalization bug that's discussed in tensorflow/tfjs#2170 that was later fixed in tensorflow/tfjs#2174, which was released in a version of tfjs higher than the one that we've been using.

Once #846 is merged, I'll rebase these branches and see if it helps!

@bomanimc bomanimc force-pushed the bomani.add-examples branch from 78fd1e3 to c562b2b Compare March 31, 2020 14:05
bomanimc added 20 commits March 31, 2020 10:07
@bomanimc bomanimc force-pushed the bomani.examples-monorepo-test branch from 4202f82 to 9662522 Compare March 31, 2020 14:19
@bomanimc
Copy link
Member Author

Finally got the tests to pass! Merging this in now.

@bomanimc bomanimc merged commit f1d6d26 into bomani.add-examples Mar 31, 2020
@joeyklee
Copy link
Contributor

Woohoo!!! Big moves! 🙏 Super great effort here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants