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

Topology mismatch with 0.20.0.alpha releases #78

Closed
j3nsykes opened this issue Feb 23, 2024 · 6 comments · Fixed by #84
Closed

Topology mismatch with 0.20.0.alpha releases #78

j3nsykes opened this issue Feb 23, 2024 · 6 comments · Fixed by #84
Labels
bug Something isn't working

Comments

@j3nsykes
Copy link

Not wanting to open duplicate issues but I have narrowed this issue
to be related to the last 3 alpha releases of ML5.

It is not present with ml5@latest/dist/ml5.min.js

However, it seems when saving and loading the files with the neural net function in the alpha releases the tensorflow topology is expecting a different structure ?

I have tried this with nextgen handpose and also a simple mouse X,Y regression. Both cause this error as soon as the version is 0.20.0.alpha 1,2 or 3.

@shiffman
Copy link
Member

Thank you for reporting this @j3nsykes! Can you share one of your examples where you are getting this error. I'd like to take a look. I have been using the ml5.neuralNetwork() class for my new chapters in The Nature of Code, but I never actually tested the saving and loading model features!

it's absolutely fine to open duplicate issues if they are present in this new repo! I realize we've caused some extra confusion and additional labor in moving to a new repo, but it helped us re-engage with the project after a dormant period! Hopefully we'll be able to bring everything together smoothly into a new release soon!!!

In case you are curious to take a look: https://natureofcode.com/neural-networks/ https://natureofcode.com/neuroevolution/)

@j3nsykes
Copy link
Author

Hey Dan,
Sure its happening here when I'm adapting one of Golan's hand tracking regression examples to be saved and loaded but also in a simple mouse regression example too (so not related to handpose.js)

The mouse example I've been using to test which version of ML5 it breaks with.

@shiffman
Copy link
Member

Ok, so I've dug into this error a bit more! The code still works if you are loading the model files directly with a single filename rather than an object:

This works:

  classifier.load("model.json", modelLoaded);

However, this does not work:

  const modelDetails = {
    model: "model.json",
    metadata: "model_meta.json",
    weights: "model.weights.bin",
  };
  classifier.load(modelDetails, modelLoaded);

However, only the second option is compatible with the web editor where the files are stored separately and the code needs to extrapolate the hosted path (?). It does that via something along the lines of:

 this.model = await tf.loadLayersModel(tf.io.browserFiles([file1, file21]));

Rather than the more simple:

   this.model = await tf.loadLayersModel(file1);

So I think something changed in a new version of tf.js with tf.io.browserFiles(). @ziyuan-linn offered in our meeting this morning to look into this further! Hopefully this helps you get started with a place to look!

@j3nsykes
Copy link
Author

Ah amazing thank you. I actually plan to not use the web editor so can probably progress with a single filename option for now.
Thanks for digging in to this!

@ziyuan-linn ziyuan-linn added the bug Something isn't working label Mar 4, 2024
@ziyuan-linn ziyuan-linn linked a pull request Mar 4, 2024 that will close this issue
@ziyuan-linn
Copy link
Member

Thank you @j3nsykes for reporting the issue and thank you @shiffman for pointing me in the right direction! The issue should be fixed now with the new PR.

The original code passes a twice-stringified JSON file to TensorFlow.js, and it only gets parsed once by tfjs, hence the modelTopology field missing error. I have removed the JSON.stringify call on ml5's side to fix the issue.

I didn't look into it, but my best guess is that a previous version of Axios automatically parsed JSON results and no longer does so in the current version. Which is why we have the redundant JSON.stringify.

@j3nsykes
Copy link
Author

j3nsykes commented Mar 4, 2024

thank you @ziyuan-linn and @shiffman !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants