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

feat(instillmodel): implement instill model embedding #727

Merged
merged 8 commits into from
Oct 18, 2024

Conversation

chuang8511
Copy link
Contributor

@chuang8511 chuang8511 commented Oct 10, 2024

Because

  • we need embedding from instill model

This commit

  • add the standardised input & output for embedding
  • add embedding to instill model
  • support oneOf for the array items

Copy link

linear bot commented Oct 10, 2024

@chuang8511 chuang8511 force-pushed the chunhao/ins-6546-embedding branch 2 times, most recently from 3fab002 to a9c3510 Compare October 16, 2024 11:03
@chuang8511 chuang8511 changed the title feat(instillmodel): add the design of how to implement instill model embedding feat(instillmodel): implement instill model embedding Oct 16, 2024
@chuang8511 chuang8511 marked this pull request as ready for review October 16, 2024 11:08
@chuang8511
Copy link
Contributor Author

chuang8511 commented Oct 16, 2024

✅ note, There is bug in model-backend. Before we merge it, we should wait for the bug fix and do end-to-end test again. Before that, we can review it first.

It fixed.

@chuang8511 chuang8511 force-pushed the chunhao/ins-6546-embedding branch from a9c3510 to 7493ab5 Compare October 16, 2024 15:32
jvallesm
jvallesm previously approved these changes Oct 17, 2024
Copy link
Collaborator

@jvallesm jvallesm left a comment

Choose a reason for hiding this comment

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

Descriptions are a bit long but they're well-written and understandable. I added some non-blocking comments but the code looks good 👍

pkg/component/ai/embedding.go Show resolved Hide resolved
pkg/component/ai/instill/v0/embedding.go Outdated Show resolved Hide resolved
pkg/component/ai/instill/v0/main.go Show resolved Hide resolved
pkg/component/tools/compogen/pkg/gen/schema.go Outdated Show resolved Hide resolved
@chuang8511
Copy link
Contributor Author

Descriptions are a bit long.

Yes, I intend to do that.
According to web operator experience, I think now we have the tendency to write the document in this pattern. I hope it aligns with what you think.

cc @GeorgeWilliamStrong

jvallesm
jvallesm previously approved these changes Oct 18, 2024
Copy link
Collaborator

@jvallesm jvallesm left a comment

Choose a reason for hiding this comment

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

Yes, I intend to do that.
According to web operator experience, I think now we have the tendency to write the document in this pattern. I hope it aligns with what you think.

Yes, no issues with it. I just noticed different from other components, where we tend to be more concise. But if it responds to a need we've observed, as long as descriptions are clear and well-written, I'm for it.

There is a conflict that should be resolved when generating the README doc. Pre-approving as the code is ready to be merged.

@donch1989 donch1989 force-pushed the chunhao/ins-6546-embedding branch from 81fd8f7 to 742605d Compare October 18, 2024 17:05
@donch1989 donch1989 merged commit 17d88bc into main Oct 18, 2024
11 checks passed
@donch1989 donch1989 deleted the chunhao/ins-6546-embedding branch October 18, 2024 17:08
donch1989 pushed a commit that referenced this pull request Oct 22, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.44.0-beta](v0.43.2-beta...v0.44.0-beta)
(2024-10-22)


### Features

* **collection:** add concat
([#748](#748))
([04d1467](04d1467))
* **compogen:** improve Title Case capitalization
([#757](#757))
([f956ead](f956ead))
* **component:** update documentation URL to component ID
([#749](#749))
([d4083c2](d4083c2))
* **instillmodel:** implement instill model embedding
([#727](#727))
([17d88bc](17d88bc))
* **run:** run logging data list by requester API
([#730](#730))
([e1e844b](e1e844b))
* **slack:** enable OAuth 2.0 integration
([#758](#758))
([8043dc5](8043dc5))
* standardize the tag naming convention
([#767](#767))
([fd0500f](fd0500f))
* **web:** refactor web operator
([#753](#753))
([700805f](700805f))


### Bug Fixes

* **groq:** use credential-supported model in example
([#752](#752))
([fc90435](fc90435))
* **run:** not return minio error in list pipeline run
([#744](#744))
([4d0afa1](4d0afa1))
* **whatsapp:** fix dir name
([#763](#763))
([029aef9](029aef9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
jvallesm pushed a commit that referenced this pull request Oct 29, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.44.0-beta](v0.43.2-beta...v0.44.0-beta)
(2024-10-22)


### Features

* **collection:** add concat
([#748](#748))
([04d1467](04d1467))
* **compogen:** improve Title Case capitalization
([#757](#757))
([f956ead](f956ead))
* **component:** update documentation URL to component ID
([#749](#749))
([d4083c2](d4083c2))
* **instillmodel:** implement instill model embedding
([#727](#727))
([17d88bc](17d88bc))
* **run:** run logging data list by requester API
([#730](#730))
([e1e844b](e1e844b))
* **slack:** enable OAuth 2.0 integration
([#758](#758))
([8043dc5](8043dc5))
* standardize the tag naming convention
([#767](#767))
([fd0500f](fd0500f))
* **web:** refactor web operator
([#753](#753))
([700805f](700805f))


### Bug Fixes

* **groq:** use credential-supported model in example
([#752](#752))
([fc90435](fc90435))
* **run:** not return minio error in list pipeline run
([#744](#744))
([4d0afa1](4d0afa1))
* **whatsapp:** fix dir name
([#763](#763))
([029aef9](029aef9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants