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

transformers.js sample #581

Merged
merged 7 commits into from
Jul 17, 2024
Merged

transformers.js sample #581

merged 7 commits into from
Jul 17, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Jul 17, 2024

  • A new document was added under /docs/src/content/docs/guides named transformers-js.mdx, which provides a guide on using the Transformers.js library. This new file contains installation instructions, how to invoke the pipeline, and other useful information. 📚
  • A new script was added called summary-with-transformers.genai.mjs in the /packages/sample/genaisrc directory. The script utilizes the Transformers.js library for creating summaries of content. 📜
  • Update in nodehost.ts allows verbose logging for model pulling from Ollama, improving model download tracing. 🎛️
  • The method importPrompt in importprompt.ts was refactored. Instead of tracing on every import, tracing now only occurs once per module. 🔄
  • In runpromptcontext.ts, verbose logging was introduced in the createChatTurnGenerationContext function. Now, tracing will also output verbose logs for better visibility. ✨
  • The package.json file in the /packages/sample directory was also updated with a new dev dependency @xenova/transformers to support the use of Transformers.js library. 🔧

Please note that there were many minor changes regarding import statements across various files, and those are not included in this summary as per the instructions.

generated by pr-describe

const onImport = (file: string) => {
trace?.itemValue("📦 import", file)
//trace?.itemValue("📦 import", fileURLToPath(file))
}

Choose a reason for hiding this comment

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

You have commented out the trace function call. This might lead to loss of important debugging information. Please ensure this is intentional. 🕵️‍♀️

generated by pr-review-commit commented_code

}
onImport(modulePath)

Choose a reason for hiding this comment

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

The onImport(modulePath) function call seems unnecessary as it is called before the register function where it should be used. This might lead to unexpected behavior. 🧐

generated by pr-review-commit unnecessary_code

const { tsImport, register } = await import("tsx/esm/api")
unregister = register({ onImport })
const module = await tsImport(modulePath, {
parentURL,
//tsconfig: false,
onImport,
})

Choose a reason for hiding this comment

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

The onImport function is no longer passed as an argument to tsImport. This might cause the import process to not behave as expected. Please ensure this is the intended behavior. 🚀

generated by pr-review-commit missing_argument

Copy link

The changes involve adjusting how imports are handled and traced in a TypeScript file. It appears the developer is testing a different approach to tracing by commenting out some lines and adding new ones. Here are some potential concerns:

  1. The trace for the import event was commented out. This could reduce visibility into the import events if the tracing mechanism is used for debugging or logging purposes. If this change was not intentional or is part of an incomplete refactor, it could be a concern. 🔍

  2. An import event (onImport(modulePath)) is fired before registering the onImport callback function. This could potentially lead to a situation where the first import is not traced. 🧐

  3. A comment was removed from the tsImport invocation. If the 'tsconfig: false' line was a placeholder for future work or a reminder, this could be a potential concern. 📝

Here is a diff with some improvements, if the original functionality needs to be maintained:

diff --git a/packages/core/src/importprompt.ts b/packages/core/src/importprompt.ts
index 1d280a6..ceaadf7 100644
--- a/packages/core/src/importprompt.ts
+++ b/packages/core/src/importprompt.ts
@@ -2,7 +2,7 @@ import { assert } from "console"
 import { host } from "./host"
 import { logError } from "./util"
 import { TraceOptions } from "./trace"
-import { pathToFileURL } from "url"
+import { fileURLToPath, pathToFileURL } from "url"
 
 function resolveGlobal(): any {
     if (typeof window !== "undefined")
@@ -46,16 +46,14 @@ export async function importPrompt(
             import.meta.url ??
             pathToFileURL(__filename ?? host.projectFolder()).toString()
 
         trace?.itemValue(`import`, `${modulePath}, parent: ${parentURL}`)
         const onImport = (file: string) => {
            trace?.itemValue("📦 import", fileURLToPath(file))
         }
         onImport(modulePath)
         const { tsImport, register } = await import("tsx/esm/api")
         unregister = register({ onImport })
         const module = await tsImport(modulePath, {
             parentURL,
             onImport,
         })
         const main = module.default
         if (typeof main === "function") await main(ctx0)

Please verify if the changes made are as per the required functionality.

generated by pr-review

@@ -73,6 +73,7 @@ class ModelManager implements ModelService {
if (provider === MODEL_PROVIDER_OLLAMA) {
if (this.pulled.includes(modelid)) return { ok: true }

logVerbose(`ollama: pulling ${modelid}...`)

Choose a reason for hiding this comment

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

The logVerbose function is called without any context. It might be better to provide more information about the current state of the application. 🤔

generated by pr-review-commit log_without_context

}
onImport(modulePath)

Choose a reason for hiding this comment

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

The onImport function is called with modulePath but the result is not used or stored. This could lead to unexpected behavior. 😕

generated by pr-review-commit unused_function_call

if (line) trace.log(line)
if (line) {
trace.log(line)
logVerbose(line)

Choose a reason for hiding this comment

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

The logVerbose function is called with the same argument as trace.log. This could lead to duplicate log entries. 🙃

generated by pr-review-commit duplicate_logging

@pelikhan
Copy link
Member Author

/genai-describe

if (line) trace.log(line)
if (line) {
trace.log(line)
logVerbose(line)

Choose a reason for hiding this comment

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

The logVerbose function is called inside the trace.log condition. This could lead to excessive logging and performance issues. Consider moving it outside the condition or adding additional checks. 😮

generated by pr-review-commit log_in_trace

@@ -0,0 +1,80 @@
---
title: Transformer.js

Choose a reason for hiding this comment

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

The filename Transformer.js should be transformers.js to match the library's actual name.

generated by pr-docs-review-commit filename_incorrect

import sampleSrc from "../../../../../packages/sample/genaisrc/summary-with-transformers.genai?raw"


HuggingFace [Transformers.js](https://huggingface.co/docs/transformers.js/index) is a JavaScript library

Choose a reason for hiding this comment

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

The URL provided for Transformers.js documentation is incorrect; it should be https://huggingface.co/transformers/.

generated by pr-docs-review-commit incorrect_url

that lets you run pretrained models locally on your machine. The library uses [onnxruntime](https://onnxruntime.ai/)
to leverage the CPU/GPU capabilities of your hardware.

In this guide, we will show how to create [summaries](https://huggingface.co/tasks/summarization) using the [Transformers.js](https://huggingface.co/docs/transformers.js/api/pipelines#module_pipelines.SummarizationPipeline) library.

Choose a reason for hiding this comment

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

The URL provided for Transformers.js summarization pipeline is incorrect; it should be https://huggingface.co/transformers/main_classes/pipelines.html#transformers.SummarizationPipeline.

generated by pr-docs-review-commit incorrect_url


:::tip

Transformers.js has an extensive list of tasks available. This guide will only cover one but checkout their [documentation](https://huggingface.co/docs/transformers.js/pipelines#tasks)

Choose a reason for hiding this comment

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

The URL provided for Transformers.js tasks documentation is incorrect; it should be https://huggingface.co/transformers/task_summary.html.

generated by pr-docs-review-commit incorrect_url


## Installation

Following the [installation instructions](https://huggingface.co/docs/transformers.js/installation),

Choose a reason for hiding this comment

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

The URL provided for Transformers.js installation instructions is incorrect; it should be https://huggingface.co/transformers/installation.html.

generated by pr-docs-review-commit incorrect_url

## Installation

Following the [installation instructions](https://huggingface.co/docs/transformers.js/installation),
we add the [@xenova/transformers](https://www.npmjs.com/package/@xenova/transformers) to the current project.

Choose a reason for hiding this comment

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

The package name @xenova/transformers is incorrect; it should be @huggingface/transformers.

generated by pr-docs-review-commit incorrect_package_name

we add the [@xenova/transformers](https://www.npmjs.com/package/@xenova/transformers) to the current project.

```bash
npm install @xenova/transformers

Choose a reason for hiding this comment

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

The package name @xenova/transformers is incorrect in the installation command; it should be npm install @huggingface/transformers.

generated by pr-docs-review-commit incorrect_package_name

You can also install this library globally to be able to use on any project

```bash "-g"
npm install -g @xenova/transformers

Choose a reason for hiding this comment

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

The package name @xenova/transformers is incorrect in the global installation command; it should be npm install -g @huggingface/transformers.

generated by pr-docs-review-commit incorrect_package_name

You can specify a model name or let the library pick the latest and greatest.

```js
import { pipeline } from "@xenova/transformers"

Choose a reason for hiding this comment

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

The import statement uses an incorrect package name @xenova/transformers; it should be @huggingface/transformers.

generated by pr-docs-review-commit incorrect_package_import


:::note[Migrate your script to `.mjs`]

To use the `Transformers.js` library, you need to use the `.mjs` extension for your script (or `.mts` for TypeScript support).

Choose a reason for hiding this comment

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

The advice to migrate scripts to .mjs is incorrect and not necessary for using the Transformers.js library.

generated by pr-docs-review-commit incorrect_extension_advice

The example below generates a summary of each input file
before letting the model generate a full summary.

<Code

Choose a reason for hiding this comment

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

The file reference transformers.genai.mjs is incorrect; it should match the actual file name used in the project.

generated by pr-docs-review-commit incorrect_file_reference

@@ -0,0 +1,80 @@
---
title: Transformer.js

Choose a reason for hiding this comment

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

The title field in the metadata section should be 'Transformers.js' instead of 'Transformer.js'.

generated by pr-docs-review-commit metadata_field_incorrect

You can specify a model name or let the library pick the latest and greatest.

```js
import { pipeline } from "@xenova/transformers"

Choose a reason for hiding this comment

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

The import statement should use '@huggingface/transformers' instead of '@xenova/transformers'.

generated by pr-docs-review-commit incorrect_import

if (line) trace.log(line)
if (line) {
trace.log(line)
logVerbose(line)

Choose a reason for hiding this comment

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

The logVerbose function is called right after trace.log with the same argument. This could lead to duplicate logs. Consider removing one of them to avoid unnecessary duplication. 🔄

generated by pr-review-commit log_duplication

@@ -36,7 +36,7 @@ See [configuration](/genaiscript/getting-started/configuration).

`run` takes one or more [glob](https://en.wikipedia.org/wiki/Glob_(programming)) patterns to match files in the workspace.

```npx sh
```bash sh

Choose a reason for hiding this comment

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

The code fence should be just bash instead of bash sh.

generated by pr-docs-review-commit incorrect_code_fence

@pelikhan pelikhan merged commit afe4cdc into main Jul 17, 2024
9 checks passed
@pelikhan pelikhan deleted the transformers-docs branch July 17, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant