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: Implement DALL-E image generation for OpenAI Bridge #178

Merged
merged 5 commits into from
Jan 6, 2025

Conversation

DZunke
Copy link
Contributor

@DZunke DZunke commented Dec 31, 2024

WDYT? Dall-E 3 and Dall-E 2 have the same endpoint for image generation but differ a bit in input and output. I tried to combine it, even i think on the long run, when OpenAI is not deciding to bring Dall-E 3 nearer to Dall-E 2 features, it would profit from splitting the image generation models in their specific classes for type safety but for now it should be fine, from my PoV.

It is not urgent from my side to have this feature, as i already utilize it in a slimmer version in my own code. So, if there is something to think about, just let discuss if there are general ideas for improvements here 😸

@DZunke DZunke changed the title Implement Dall-E image generation feat: Implement Dall-E image generation for OpenAI Bridge Dec 31, 2024
@DZunke DZunke force-pushed the dall-e-image-generation branch from 2c0a5d3 to 83b3805 Compare December 31, 2024 11:04
@DZunke DZunke force-pushed the dall-e-image-generation branch from 83b3805 to fc88903 Compare December 31, 2024 11:05
@chr-hertel
Copy link
Member

Thanks @DZunke! Looks pretty good in first glance, will give it a try later - thanks :)

@OskarStark
Copy link
Contributor

I would also say it looks good, but didn't test it

@OskarStark OskarStark changed the title feat: Implement Dall-E image generation for OpenAI Bridge feat: Implement DALL-E image generation for OpenAI Bridge Jan 2, 2025
@chr-hertel
Copy link
Member

I guess on detailed level Oskar already made some good remarks, taking one step back my first concerns would be more about OOP slicing and naming.

In general GenAI is about multi modal content input and output and I wonder what your perspective is about promoting the PhpLlm\LlmChain\Model\Message\Content one level to PhpLlm\LlmChain\Model\Content and also using it with the response instances - we already have an image class there.

@DZunke
Copy link
Contributor Author

DZunke commented Jan 3, 2025

I guess on detailed level Oskar already made some good remarks, taking one step back my first concerns would be more about OOP slicing and naming.

In general GenAI is about multi modal content input and output and I wonder what your perspective is about promoting the PhpLlm\LlmChain\Model\Message\Content one level to PhpLlm\LlmChain\Model\Content and also using it with the response instances - we already have an image class there.

From my PoV the ImagesResponse is very special to Dall-E model because it contains the revised_prompt output made by Dall-E 3 as the model rewrites the given user prompt and it is a worth information what prompt was really taken into account for creating the image. WDYT?

@chr-hertel
Copy link
Member

is very special to Dall-E model because it contains the revised_prompt output made by Dall-E 3 as the model rewrites the given user prompt

fair, and we can generalize later - but on the first research this is pretty unique to DallE, true

@chr-hertel chr-hertel mentioned this pull request Jan 5, 2025
4 tasks
@DZunke DZunke force-pushed the dall-e-image-generation branch from 664c839 to 3655afd Compare January 6, 2025 15:20
@OskarStark OskarStark added the enhancement New feature or request label Jan 6, 2025
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Thanks @DZunke - nice addition :)

@chr-hertel chr-hertel merged commit af5e00d into php-llm:main Jan 6, 2025
7 checks passed
@OskarStark
Copy link
Contributor

nice addition :)

Yes, Thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants