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

add basic wasm support #122

Merged
merged 23 commits into from
Nov 25, 2023
Merged

Conversation

ifsheldon
Copy link
Contributor

This is based on #120 and #121.

To summarize:

  • Added two feature gates enable_tokio and enable_backoff
    • backoff is somehow difficult to make it support wasm, so I just decided to gate it under a feature
  • A minor API breaking change. Stream related functions now return OpenAIEventStream but it should not matter as long as users only use it in while let await loops since it also implements Stream
  • Updated reqwest-eventsource

This should close #102 unless file-related ops are wanted.

I tested the code by
cargo build --target wasm32-unknown-unknown --no-default-features --features wasm and an web app.

The code is

// main.rs
use async_openai::types::{ChatCompletionRequestMessageArgs, CreateChatCompletionRequestArgs, Role};
use dioxus::prelude::*;
use futures::stream::StreamExt;
use anyhow::Result;

const API_BASE: &str = "...";
const API_KEY: &str = "...";
const API_VERSION: &str = "...";
const DEPLOYMENT_ID: &str = "...";

pub fn app(cx: Scope) -> Element {
    let ok_count = use_state(cx, || 0_usize);
    let err_count = use_state(cx, || 0_usize);
    let response_string: &UseRef<String> = use_ref(cx, String::new);
    let fetch_completion_chunks: &Coroutine<()> = use_coroutine(cx, |rx| {
        let ok_count = ok_count.to_owned();
        let err_count = err_count.to_owned();
        let response_string = response_string.to_owned();
        async move {
            let config = async_openai::config::AzureConfig::new()
                .with_api_base(API_BASE)
                .with_api_key(API_KEY)
                .with_api_version(API_VERSION)
                .with_deployment_id(DEPLOYMENT_ID);
            let client = async_openai::Client::with_config(config);
            let request = CreateChatCompletionRequestArgs::default()
                .max_tokens(512u16)
                .model("gpt-3.5-turbo-0613")
                .messages([ChatCompletionRequestMessageArgs::default()
                    .role(Role::User)
                    .content("Hello!")
                    .build().unwrap()])
                .build().unwrap();
            let mut stream = client.chat().create_stream(request).await.unwrap();
            while let Some(chunk) = stream.next().await {
                match chunk {
                    Ok(response) => {
                        ok_count.modify(|x| *x + 1);
                        response_string.with_mut(|old| {
                            old.push('\n');
                            old.push_str(format!("{:?}", response).as_str());
                        })
                    }
                    Err(e) => {
                        err_count.modify(|x| *x + 1);
                    }
                }
            }
        }
    });

    render! {
        div {
            p {
                "{response_string.read()}"
            }
            p {
                "ok_count: {ok_count.get()}"
            }
            p {
                "err_count: {err_count.get()}"
            }
        }
    }
}

fn dioxus_main() {
    dioxus_web::launch(app);
}

async fn async_openai_main() -> Result<()>  {
    let config = async_openai::config::AzureConfig::new()
        .with_api_base(API_BASE)
        .with_api_key(API_KEY)
        .with_api_version(API_VERSION)
        .with_deployment_id(DEPLOYMENT_ID);
    let client = async_openai::Client::with_config(config);
    let request = CreateChatCompletionRequestArgs::default()
        .max_tokens(512u16)
        .model("gpt-3.5-turbo-0613")
        .messages([ChatCompletionRequestMessageArgs::default()
            .role(Role::User)
            .content("Hello!")
            .build()?])
        .build()?;
    let mut stream = client.chat().create_stream(request).await?;
    while let Some(chunk) = stream.next().await {
        println!("{:?}", chunk);
    }
    Ok(())
}

// for tokio testing
#[tokio::main]
async fn main() -> Result<()> {
    async_openai_main().await
}

// for wasm testing
// fn main() {
//     dioxus_main();
// }

Cargo.toml

[package]
name = "wasm_test"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
dioxus = "~0.4"
dioxus-web = "~0.4"
futures = "0.3.28"
reqwest = { version = "0.11", features = ["json"] }
reqwest-eventsource = "0.5"
# for wasm
#async-openai = { path = "../async-openai/async-openai", default-features = false, features = ["wasm"] }
serde_json = "~1.0"
serde = { version = "1.0", features = ["derive"] }
anyhow = "~1.0"
# for native async-openai
async-openai = { path = "../async-openai/async-openai" }
tokio = { version = "1.32", features = ["full"] }

@64bit
Copy link
Owner

64bit commented Oct 17, 2023

Hi @ifsheldon

Thank you for all of your good work!

This needs some work on documentation:

  1. A self contained example so its easy for me to test (like other examples) and also acts as documentation of the feature.
    Perhaps examples/azure-wasm or something with wasm in it - for the example that you have put in the description?
  2. wasm feature description on README for wasm to show on crates.io and GitHub
  3. A brief wasm section in lib.rs below Azure docs, to show on docs.rs for the crate

I also have few questions:

  • Perhaps a bit intro/docs on feature flags would help new folks too?
  • What's the rationale behind introducting OpenAIEventStream ?
  • The original WASM request was for platforms like Cloudflare Workers, would this work? Perhaps add another self contained example?
  • It is not clear which APIs are supported by this PR in wasm and which aren't unless someone goes through code.

Regarding your note about testing on OpenAI in the other PR - I'll be happy to help test changes and let you know how it goes - please expect some delay from my side though.

@ifsheldon
Copy link
Contributor Author

ifsheldon commented Oct 17, 2023

For documentation, I will add more when I have some time.

Perhaps a bit intro/docs on feature flags would help new folks too?

Sure, but I don't know much about the feature flags that existing in main now, tls related ones.

What's the rationale behind introducting OpenAIEventStream ?

  1. To get rid of tokio.spawn and further get rid of tokio. Lik in add a struct to transform EventSource stream into OpenAI response stream #121, I think what you did just transforming a stream into another one, but you did it using two tasks (a tx task and a rx task), which needs tokio.spawn. I can't just use stream filtering and mapping mainly because if message.data == "[DONE]" branch which is early stoping a stream.
  2. Choosing to return OpenAIEventStream is another story, but, simply because I couldn't win the fighting with rustc. You can try reverting 0126c6a but you will see compile errors like
     Box::pin(OpenAIEventStream::new(event_source))
        |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `*mut u8` cannot be sent between threads safely
    
    which seem to appear from nowhere. As I understand, you were trying to return an opaque dyn trait object that implements Stream. But I don't think this opaqueness is necessary as long as OpenAIEventStream also impls Stream.

The original WASM request was for platforms like Cloudflare Workers, would this work? Perhaps add another self contained example?

I think it will work nonetheless. Haven't tries these though. As far as I know wasm32-unknown-unknown is the minimum target in wasm family, which basically targets any web browsers. Runtimes or platforms of wasm other than web browsers support more functionalities and targets like wasm-wasi. The differences between wasm32-unknown-unknown and wasm-wasi see https://users.rust-lang.org/t/wasm32-unknown-unknown-vs-wasm32-wasi/78325 I think as long as this compiles for wasm32-unknown-unknown, any wasm runtimes or platforms can work.

It is not clear which APIs are supported by this PR in wasm and which aren't unless someone goes through code.

Basically anything related to files is not supported, except finetuning since it only needs strings in the code, not directly related to files. I will write more in the documentation, though. Future work can be removing Pathbuf from data structures so that any structs related to media hold bytes or byte streams, then web developers can use web technologies to upload media first.

@ifsheldon
Copy link
Contributor Author

@64bit I've added documentation and an example. Can you review these? Thanks!

@64bit
Copy link
Owner

64bit commented Oct 22, 2023

Thank you for updates, please expect some delays as previously mentioned, to provide you context - some older PRs & spec update issues needs attention and given that this is a big new feature and requires testing - it might be a while before I get to this. In the meantime would you mind closing your other PRs which are no longer relevant? Thank you for your patience.

@ifsheldon
Copy link
Contributor Author

@64bit do you have any plan to merge this? or any comments? I saw new features got added recently, which makes this PR more intertwined and need more testing. If you have no intention to merge this, I can close this. Or I can try to follow up new features and see if they complies on wasm.

@64bit
Copy link
Owner

64bit commented Nov 8, 2023

Hi @ifsheldon ,

The primary & bare minimum purpose of this crate is to work with OpenAI API - if it doesn't work with API this crate should not exist - that's why some PRs were merged before any other open PRs.

That said, I'm sure community would love to have wasm support and I do too - however I'm just limited by my bandwidth. This PR is a big feature and requires testing and doubles the surface area for testing (wasm and non-wasm) - without breaking existing features - all that creates extra work for me for every single update now and in the future.

As much as I want to merge your contributions, above are the practical reasons that I cannot accept this PR anytime soon, I'm not sure when I could spend time on this, and so I'm really sorry about that.

It would nice to have your work in this PR published - so here are few options:

  1. We release alpha version from this branch - but you take full ownership of maintaining the branch upto date with main - I'll just be publising alpha versions or beta, stable as they mature. [ This would still be limited by my bandwidth ]
  2. Fork async-openai crate and create a new wasm only crate something like async-openai-wasm that way you get maximum flexibility for maintaining, updating and publishing to crates.io.

Please let me know what you think and any alternative path forward I'm open to hear them.

Thank you

@ifsheldon
Copy link
Contributor Author

OK. Either way I need to maintain the code, so I'd rather not to distract developers by forking a new crate. So I think 1. is good for me. I will try to keep up with OpenAI new features soon. When it's done, you can just release an alpha.

@64bit
Copy link
Owner

64bit commented Nov 11, 2023

Thank you for your willingness!

Let's ship it, I'm thinking to create 'experiments' branch to merge into and other related future changes

# Conflicts:
#	async-openai/Cargo.toml
#	async-openai/src/client.rs
#	async-openai/src/lib.rs
#	async-openai/src/types/impls.rs
#	async-openai/src/types/types.rs
@ifsheldon
Copy link
Contributor Author

ifsheldon commented Nov 19, 2023

@64bit Great! I've made few changes to get features in this branch in sync with main. My example has been updated to support OpenAI APIs, and I've tested it with Azure OpenAI and OpenAI.

The next step may be to separate file paths from file binaries in Input structs. Like ImageInput has a path field which is not nice to wasm.

@64bit 64bit changed the base branch from main to experiments November 25, 2023 03:38
@64bit 64bit merged commit 92650e4 into 64bit:experiments Nov 25, 2023
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.

Wasm support
2 participants