TheComamba / UnKenny

A FoundryVTT module providing NPCs with artificial intelligence.
MIT License
3 stars 5 forks source link

Re-assess local models and how they are called #53

Closed TheComamba closed 6 months ago

TheComamba commented 7 months ago

Before going for Version 1.0, I think we should check if we are using the local models properly. Are there better models available? How can we call the model to be a chatbot instead of a mere text completion model?

TheComamba commented 7 months ago

The tokenizer object knows a function named apply_chat_template, which can be called with an array of messages exactly as the OpenAI api is called. However, many models do not seem to provide such a chat template. I have wrapped the call in a function that listens for a warning and prints it as an error message in Foundry. Without such a default template, the output is pretty useless. The current issue is therefore to find models that provide such a template.

On another node: I would really like to test the local models on a regular basis, but because Jest is not made for ES modules, it cannot use the import statement and thus has no access to transformers.js. This makes maintaining the local models... challenging.

TheComamba commented 7 months ago

Update: I managed to enable testing ES modules with jest. :) The changes will enter main with the next merge.

TheComamba commented 6 months ago

We are currently getting an error: An error occurred during model execution: "TypeError: A float32 tensor's data must be type of function Float32Array() { [native code] }". The callback is:

      at new h (node_modules/onnxruntime-common/dist/webpack:/onnxruntime-common/lib/tensor-impl.ts:111:17)
      at m.run (node_modules/onnxruntime-common/dist/webpack:/onnxruntime-common/lib/inference-session-impl.ts:112:28)
      at sessionRun (node_modules/@xenova/transformers/src/models.js:207:22)
      at Function.decoderForward [as _forward] (node_modules/@xenova/transformers/src/models.js:547:26)
      at Function.forward (node_modules/@xenova/transformers/src/models.js:820:16)
      at Function.decoderRunBeam [as _runBeam] (node_modules/@xenova/transformers/src/models.js:633:18)
      at Function.runBeam (node_modules/@xenova/transformers/src/models.js:1373:16)
      at Function.generate (node_modules/@xenova/transformers/src/models.js:1087:30)
      at getResponseFromLocalLLM (src/scripts/local-llm.js:74:18)
      at Object.<anonymous> (src/scripts/local-llm.test.js:20:26)

The output of tokenier() is an Array of Bigint64. I have tried not returning a tensor object (per options argument), and I have tried converting the tensor to float (no idea if what I tried was right), both to no avail.

TheComamba commented 6 months ago

I learned that this probably has to do with us transpiling the ECMA Script to CommonJS with Babel, because CommonJS does not support BigInt. But well, the tokenizer is giving us BigInts. This also explains why the behaviour seems to be restricted to the test environment and appeared only recently. Do I know how to fix this? Nope. Not yet. Man, this programming language...

TheComamba commented 6 months ago

The BigInt64Array is a built in type: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt64Array The previous values are added with a hard coded float32 type: https://github.com/xenova/transformers.js/blob/main/src/models.js (Function addPastKeyValues, Line 1346) The error message is thrown in the constructor of a new tensor: https://github.com/microsoft/onnxruntime/blob/main/js/common/lib/tensor-impl.ts (Line 174) This throw can only be reached if Array.isArray(arg1) in Line 144 evaluates to false. The argument here is the BigInt64Array, which is a TypedArray, and thus not an actual Array, so the expected evaluation is indeed false: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/isArray The constructor is called here: https://github.com/microsoft/onnxruntime/blob/main/js/common/lib/inference-session-impl.ts (Line 118) The second argument is indeed result.data, which in my debugger for the "logits" key has Type Float32Array.

TheComamba commented 6 months ago

The check if (arg1 instanceof typedArrayConstructor) in https://github.com/microsoft/onnxruntime/blob/main/js/common/lib/tensor-impl.ts, Line 171 fails. According to Copilot: """ Yes, it could be related to the transpilation process.

When you transpile ES modules to CommonJS using Babel, it can sometimes cause issues with instanceof checks. This is because Babel's transpilation process can create copies of constructors like Float32Array, which means the instanceof check may fail even if arg1 is what you would consider an instance of Float32Array.

In other words, arg1 might be an instance of a different Float32Array constructor than typedArrayConstructor, even though they are conceptually the same thing. This is a known issue when working with transpiled code and instanceof checks. """

TheComamba commented 6 months ago

I tried fixing it with a babel plugin: https://github.com/TheComamba/UnKenny/commit/25f350eb2ca4eda719bc8f8bfb5a7a47e7b200a1 So far, no success, but maybe this is progress?

TheComamba commented 6 months ago

I tried converting the whole shebang to a CommonJS model. https://github.com/TheComamba/UnKenny/tree/dev/treating-code-as-common-js So far, I failed. Importing transformers seems to be a really bad idea when using Jest. Another hurdle is that requiring from an URL is not allowed. We can download the module and then require it locally. In theory. In practice I did not manage to do that, because this introduces an asynchronicity that I failed to resolve in my frustrated state of mind.

TheComamba commented 6 months ago

I think it might be time to consider moving to another test framework: https://github.com/traceypooh/js-es-modules-coverage-testing

TheComamba commented 6 months ago

I started doing that: https://github.com/TheComamba/UnKenny/tree/dev/mocha-for-tests