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

Supporting p5's preload() for loading models #92

Closed
shiffman opened this issue Mar 6, 2018 · 16 comments
Closed

Supporting p5's preload() for loading models #92

shiffman opened this issue Mar 6, 2018 · 16 comments

Comments

@shiffman
Copy link
Member

shiffman commented Mar 6, 2018

For beginners it's wonderful to support p5's preload(). I noticed some of our examples already include preload() for loading models. This may, in fact, be working just fine but I am opening an issue as a reminder to check if they are functioning correctly and that we are conforming to the p5 preload system. Here are some relevant links:

I think we would need something in the ml5 library that checks for the existence of p5 and if it does exists registers our methods with registerPreloadMethod()?

Apologies if this is in the codebase somewhere already I searched and didn't see it.

@cvalenzuela
Copy link
Member

We are not supporting p5 preload method but all examples do work with p5 at the moment. Either by setting this up the ml5 methods before or inside the setup() or preload() methods.

Not sure if we need to implement a custom register for the preload(). @shiffman?

@shiffman
Copy link
Member Author

Looking at the Image Classifier example does ml5.imageClassifier('Mobilenet'); not require a callback to ensure the model is ready? Does it work simply because it takes a bit for the image to be ready so the model also happens to be ready? I am imagining a imageClassifier example (related to ml5js/ml5-examples#7) like so:

let classifier;
let img;

function preload() {
  classifier = ml5.imageClassifier('Mobilenet');
  img = loadImage('bird.jpg');
}


function setup() {
  createCanvas(400, 400);
  image(bird, 0, 0);
  classifier.predict(img, gotResult);
}

function gotResult(results) {
  createPl(results[0].className);
}

@shiffman
Copy link
Member Author

shiffman commented Jun 13, 2018

The LSTM example is another one where we could consider supporting preload()?

let lstm;

function preload() {
  lstm = ml5.LSTMGenerator('models/nietschze/');
}

function setup() {
  noCanvas();
  lstm.generate({ seed: 'Hello, my name is' }, gotResult);
}

function gotData(result) {
  createP(result);
}

@cvalenzuela
Copy link
Member

It does support a callback. See https://ml5js.org/docs/ImageClassifier#syntax

I'm going to investigate a more formal integration with p5

@marshalhayes
Copy link
Contributor

Any update on support for p5's preload()? I would like to work on this if you don't mind, but might need some guidance on where to start.

@NHibiki
Copy link
Member

NHibiki commented Feb 6, 2019

I think if p5 is already loaded, we can just need to make sure that the preload() is compatible with ml5.

If we are doing standalone preload() just like what p5 does. Should we create an element and insert into document to give users a hint that some components are going to be loaded? It is a bit weird since ml5 is not a visualization library.

btw, in LSTM Example Page, there is an error that ml5.LSTMGenerator is not a function. Is it because the example does not use the latest verion of ml5?

@shiffman
Copy link
Member Author

shiffman commented Feb 6, 2019

Hi @NHibiki, the idea here is not to create a new preload() function for ml5, but rather to alert p5 to the existence of ml5 so that our model loading functions can be placed in preload() and work accordingly (finish before setup()).

There is information in the p5 developer docs here: Use registerPreloadMethod() to register names of methods with p5 that may be called in preload().

Re: LSTM the name changed to charRNN and we've forgotten to update it in a lot of places. You can file a separate issue in ml5-examples or ml5-website depending on where the mistake is. Thank you!

@marshalhayes
Copy link
Contributor

I'm happy to see this moving along. p5 and ml5 should go hand-in-hand, and having to deal with model loading outside of the usual p5 functions isn't always easy for new programmers.

@NHibiki
Copy link
Member

NHibiki commented Feb 7, 2019

@shiffman Wow, I have read the document about registerPreloadMethod. It is very comprehensive and well documented. With the help of the function, we do allow users to use an easier way to load data or initialize before moving on when they are using both p5 and ml5.

But would there be a scenario that some users only use ml5 in their code but still want a simple way to preload?

@shiffman
Copy link
Member Author

shiffman commented Feb 7, 2019

Hi @NHibiki, I think if someone is using vanilla JavaScript without p5 then the standard asynchronous methodology for loading a model will be enough. The concept of preload() is pretty unique to p5 and not a part of other JavaScript patterns I've seen. Thank you!

@EmmaGoodliffe
Copy link
Contributor

@NHibiki @shiffman Can all ml5 models load in preload() because of #288 now?

@joeyklee
Copy link
Contributor

@EmmaGoodliffe - I think Sentiment still needs to be tested and I'm not sure if the NeuralNetwork is something that makes sense to put in preload. Let's leave this open for now until we test if Sentiment works with preload.

Screen Shot 2019-10-12 at 15 07 51

Thanks for going through all these issues!

@NHibiki
Copy link
Member

NHibiki commented Oct 13, 2019

@joeyklee after reading the implemented code of Sentiment roughly, it should support preload theoretically. Let me test it manually later.

@EmmaGoodliffe
Copy link
Contributor

This quick sketch shows that the when run with preload, the model is undefined in setup but with the callback method, it is a model object (e {...}) as you would expect. I'm not sure why though.

@shiffman
Copy link
Member Author

Thanks for this wonderful discussion and everyone's contributions! I think it makes sense to support ml5.sentiment() with preload() for sure since it is loading a pre-trained model. Since ml5.neuralNetwork() doesn't load a model (by default) it doesn't need to be in preload(). However, we could consider supporting preload() for loading the data for training or loading a model that was saved from ml5.neuralNetwork().

@joeyklee
Copy link
Contributor

Hi All!

As most of our models now abide by this standard, I will close this issue now. We can revisit this for specific model needs as they arise in the future. Thank you for your participation all!

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

No branches or pull requests

6 participants