Skip to content
This repository was archived by the owner on Sep 18, 2024. It is now read-only.

feat: add validator adapter #97

Merged
merged 14 commits into from
May 3, 2024
Merged

Conversation

tobySolutions
Copy link
Member

@tobySolutions tobySolutions commented Apr 28, 2024

Description of the pull request

Changes made

Fixes #93

  • Added validator adapter for other models from our API
  • Wrote tests to handle them
  • ....

Related issues

Testings done

Screenshots (if any)

Checklist

  • I have written tests
  • My code does not produce new errors
  • I gave myself a code review before asking others.

Copy link

vercel bot commented Apr 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
akeru-akeru-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 2, 2024 11:18pm

multipletwigs
multipletwigs previously approved these changes Apr 29, 2024
@multipletwigs multipletwigs dismissed their stale review April 29, 2024 03:04

accidental approval

@tobySolutions tobySolutions added 🟨 priority: medium Not blocking but should be fixed soon ✨ goal: improvement Improvement to an existing feature 💪 skill: api requires proficiency with apis labels Apr 29, 2024
@GuiBibeau
Copy link
Collaborator

@tobySolutions Great work on this! I'm loving it.

@multipletwigs can you check that the code in run thread is aligned with your architecture of that code?

@tobySolutions
Copy link
Member Author

@tobySolutions Great work on this! I'm loving it.

@multipletwigs can you check that the code in run thread is aligned with your architecture of that code?

Thanks Gui, really appreciate it!!!

@multipletwigs
Copy link
Collaborator

multipletwigs commented May 1, 2024

@tobySolutions i'm actually a bit confused on what we're trying to achieve with this, since Validators and Assistants are inherently different domains.

  if (assistantData.model === "llama-2-7b-chat-int8") {
    const validatorAdapterRes: any = await validatorAdapter(
      everyRoleAndContent,
      "llama-2-7b-chat-int8",
      assistantData.instruction
    );

    const assistantResponse: string =
      validatorAdapterRes.choices[0].message.content;

    // add assistant response to the thread
    await createMessage(assistant_id, thread_id, assistantResponse);

    const threadRunResponse: ThreadRun = {
      id: uuidv4(),
      assistant_id: assistant_id,
      thread_id: thread_id,
      created_at: new Date(),
    };
    return threadRunResponse;
  }
}

This can get messy very quickly when we have a lot of models to validate.

@GuiBibeau In a way yes, the architecture is kinda how a thread run should be. I ran through the validator code, and it seems like the validator will take in the original Thread content, find the fastest miner, and make a request to it.

Just to confirm my understanding, Akeru can make a request to a Validator, and ask "hey i have a thread with these messages, can anyone give me a response", and Validators will see which Miner to request from, and give the response back to Akeru. Which is different from our existing Assistants right? So it makes sense to have two different domains anyways for it.

@multipletwigs
Copy link
Collaborator

If we were to still make Bittensor validators as part of our assistants, then the best course is to just differentiate the models with bt-<insert model name> with a bt prefix, and call the validator adapter instead. Include model data as part of the api call request, and have the Validator decide what to do with it. We shouldn't check when to call Validator models with a bunch of if statements.

@GuiBibeau
Copy link
Collaborator

@tobySolutions i'm actually a bit confused on what we're trying to achieve with this, since Validators and Assistants are inherently different domains.

  if (assistantData.model === "llama-2-7b-chat-int8") {
    const validatorAdapterRes: any = await validatorAdapter(
      everyRoleAndContent,
      "llama-2-7b-chat-int8",
      assistantData.instruction
    );

    const assistantResponse: string =
      validatorAdapterRes.choices[0].message.content;

    // add assistant response to the thread
    await createMessage(assistant_id, thread_id, assistantResponse);

    const threadRunResponse: ThreadRun = {
      id: uuidv4(),
      assistant_id: assistant_id,
      thread_id: thread_id,
      created_at: new Date(),
    };
    return threadRunResponse;
  }
}

This can get messy very quickly when we have a lot of models to validate.

@GuiBibeau In a way yes, the architecture is kinda how a thread run should be. I ran through the validator code, and it seems like the validator will take in the original Thread content, find the fastest miner, and make a request to it.

Just to confirm my understanding, Akeru can make a request to a Validator, and ask "hey i have a thread with these messages, can anyone give me a response", and Validators will see which Miner to request from, and give the response back to Akeru. Which is different from our existing Assistants right? So it makes sense to have two different domains anyways for it.

@tobySolutions @multipletwigs just to confirm I love the idea of prefixing the models coming from the subnet with something like bt.

On the main issues at hand: the idea would be that the validator/miner combo can act fully on the same level as the gpt-3.5/4.0 adapter. So if somebody creates an assistant with ex: bt-mistral-7b. All prompts and thread runs would be fulfilled by bt-mistral-7b in the same way that gpt-3.5 backed assistants are fulfilled by open-ai. The subnet becomes an external provider of inference on the same footing as existing adapters.

Does that make sense?

@multipletwigs
Copy link
Collaborator

@GuiBibeau in that case, yes this makes sense now! So the Models available should be bt-gpt-4, bt-gpt-3.5, gpt-4, gpt-3.5 respectively and if it's a bt model, run it by the miner/validator adapter, else run it through the conventional adapters. This is more manageable i think.

@tobySolutions
Copy link
Member Author

Made updates with all your feedback @multipletwigs!! Thanks for everything.

Copy link
Collaborator

@multipletwigs multipletwigs left a comment

Choose a reason for hiding this comment

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

LGTM

@tobySolutions
Copy link
Member Author

LGTM

Awesome, thanks!

@tobySolutions tobySolutions merged commit 34e20a9 into main May 3, 2024
3 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✨ goal: improvement Improvement to an existing feature 🟨 priority: medium Not blocking but should be fixed soon 💪 skill: api requires proficiency with apis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Backend]: call the validator to use the available models
3 participants