ml5js / ml5-website-v02-docsify

This repository is a work-in-progress website, containing documentation and reference materials for the ml5.js library models.
https://docs.ml5js.org/
Other
1 stars 8 forks source link

"topk" documentation #147

Open shiffman opened 2 months ago

shiffman commented 2 months ago

Opening a discussion for topk related to https://github.com/ml5js/ml5-next-gen/issues/166.

  1. The documentation states "The number of top predictions to return, applicable to MobileNet." I think we can remove "applicable to MobileNet" since this is the top predictions that should be returned regardless of classification model.
  2. Should we consider our own name for this property (mapped to topk) and/or a glossary entry?
alanvww commented 2 months ago

Hello Dan, in terms of this variable, I wrote it based on our source code

switch (this.modelName) {
        case "mobilenet":
          this.modelToUse = mobilenet;
          const config = handleOptions(
            options,
            {
              version: {
                type: "enum",
                enums: [1, 2],
                default: 2,
              },
              alpha: {
                type: "enum",
                enums: (config) =>
                  config.version === 1
                    ? [0.25, 0.5, 0.75, 1.0]
                    : [0.5, 0.75, 1.0],
                default: 1.0,
              },
              topk: {
                type: "number",
                integer: true,
                default: 3,
              },
            },
            "imageClassifier"
          );
          this.version = config.version;
          this.alpha = config.alpha;
          this.topk = config.topk;
          break;
        case "darknet":
          this.version = "reference"; // this a 28mb model
          this.modelToUse = darknet;
          break;
        case "darknet-tiny":
          this.version = "tiny"; // this a 4mb model
          this.modelToUse = darknet;
          break;
        case "doodlenet":
          this.modelToUse = doodlenet;
          break;
        default:
          this.modelToUse = null;
      }
    }

My understanding is that topk is only related to MobileNet at the moment. And during my meeting with Peter this morning we realized that this variable might not be used even if it is in the code. There is topk in both DoodleNet and DarkNet and the way we have implemented it is this:

classify(img, topk = 10) {
...
}

I think this is why in Jack's problem the kNumber works in imageClassifier.classifyStart(media, ?kNumber, callback);. And I personally prefer the way of using kNumber instead of topk. It provides more space to work with, while the developer can decide the prediction number on the fly instead of setting it at the beginning and tweaking it later.

But either way! I will update the docs to reflect how we decide to implement it :)

jackbdu commented 2 months ago

Just to clarify, passing { topk: 5 } has no impact on MobileNet neither. I agree with @alanvww that it seems reasonable to specify kNumber in .classify()/.classifyStart() and not in the constructor. That also gives us consistency across different models. Also, not sure if it's worth mentioning in the documentation, but I just realized specifying kNumber in .classify()/.classifyStart() has no impact when used with a Teachable Machine model, the model always gives complete results (both of the two classes that I trained it with in my test).

shiffman commented 2 months ago

Ah, I see, I wasn't looking closely at this. I agree, we should focus on the user specifying the number of classes they want to see returned right in the classify() and classifyStart() methods. Happy to call this kNumber of whatever is most clear and friendliest to a beginner. Specifically a topK number when loading the model doesn't have much value for the ml5.js context!

shiffman commented 3 weeks ago

This has now been implemented in the code (https://github.com/ml5js/ml5-next-gen/pull/179) and we can finalize the documentation, here are some possible next steps:

  1. Remove topk model property? Or should we leave it in the documentation since it does work?
  2. Add an explanation that it can be included as option argument to classify() in the Classify the Image section?
  3. Update classifyStart() and classify() documentation. In the meeting we discussed naming this argument topk but I'm fine with keeping it as kNumber.