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.

Sign up for GitHub

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

Jump to bottom

Add Image Generation support using Imagen #8683

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Add Image Generation support using Imagen #8683

wants to merge 16 commits into from

Conversation

dlarocque
Copy link
Contributor

WIP

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Dec 17, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: bfc2c0f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

hsubox76
hsubox76 reviewed Dec 17, 2024
View reviewed changes
Copy link
Contributor

@hsubox76 hsubox76 left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just some nits (mostly about doc formatting) and small questions.

common/api-review/vertexai.api.md Show resolved Hide resolved
packages/vertexai/src/api.test.ts Outdated Show resolved Hide resolved
packages/vertexai/src/models/imagen-model.ts Outdated Show resolved Hide resolved
packages/vertexai/src/models/imagen-model.test.ts Outdated Show resolved Hide resolved
packages/vertexai/src/requests/request-helpers.ts Outdated Show resolved Hide resolved
packages/vertexai/src/requests/response-helpers.ts Outdated Show resolved Hide resolved
packages/vertexai/src/types/imagen/requests.ts Outdated
addWatermark?: boolean;
safetyFilterLevel?: string;
personGeneration?: string;
includeRaiReason: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

We put the word "rai" in the parameter going up but we remove it in the response? I can see reasons for including or not including but it should be consistent I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This property is not user-facing.

Users define configurations in ImagenGenerationConfig and ImagenModelParams, and these interfaces do not define a parameter related to the RAI reason- users cannot disable this. These configurations get mapped to this PredictRequestBody interface which is internal. Here, we set includeRaiReason to always be true, since users cannot define it in the config.

The flow looks like:
User --> ImagenGenerationConfig & ImagenModelParams --> PredictRequestBody (includeRaiReason: true)--> HTTP request --> HTTP response --> ImagenResponseInternal (raiFilteredReason: ...) --> ImagenImageResponse (filteredReason: ...) --> User

@dlarocque dlarocque changed the title WIP Add Image Generation support with Imagen Jan 3, 2025
@dlarocque dlarocque changed the title Add Image Generation support with Imagen Add Image Generation support using Imagen Jan 3, 2025
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 3, 2025 •
edited
Loading

Size Report 1

Affected Products

  • @firebase/app-check

    TypeBase (32bf021)Merge (9df0159)Diff
    browser26.3 kB26.3 kB+4 B (+0.0%)
    main27.3 kB27.3 kB+6 B (+0.0%)
    module26.3 kB26.3 kB+4 B (+0.0%)

  • @firebase/remote-config

    TypeBase (32bf021)Merge (9df0159)Diff
    browser19.2 kB21.7 kB+2.56 kB (+13.3%)
    main20.2 kB22.9 kB+2.65 kB (+13.1%)
    module19.2 kB21.7 kB+2.56 kB (+13.3%)

  • @firebase/util

    TypeBase (32bf021)Merge (9df0159)Diff
    browser23.4 kB23.2 kB-168 B (-0.7%)
    main29.2 kB29.0 kB-222 B (-0.8%)
    module23.4 kB23.2 kB-168 B (-0.7%)

  • @firebase/vertexai

    TypeBase (32bf021)Merge (9df0159)Diff
    browser28.8 kB33.0 kB+4.21 kB (+14.6%)
    main29.6 kB34.0 kB+4.32 kB (+14.6%)
    module28.8 kB33.0 kB+4.21 kB (+14.6%)

  • bundle

    TypeBase (32bf021)Merge (9df0159)Diff
    app-check (CustomProvider)37.5 kB37.3 kB-142 B (-0.4%)
    app-check (ReCaptchaEnterpriseProvider)40.0 kB39.8 kB-144 B (-0.4%)
    app-check (ReCaptchaV3Provider)39.9 kB39.8 kB-144 B (-0.4%)
    remote-config (getAndFetch)46.3 kB47.5 kB+1.20 kB (+2.6%)

  • firebase

    TypeBase (32bf021)Merge (9df0159)Diff
    firebase-app-check-compat.js23.4 kB23.2 kB-136 B (-0.6%)
    firebase-app-check.js25.0 kB24.9 kB-107 B (-0.4%)
    firebase-compat.js797 kB798 kB+996 B (+0.1%)
    firebase-remote-config-compat.js27.3 kB28.4 kB+1.13 kB (+4.1%)
    firebase-remote-config.js29.6 kB31.3 kB+1.69 kB (+5.7%)
    firebase-vertexai.js23.8 kB27.0 kB+3.15 kB (+13.2%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/utGsfdcsOA.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 3, 2025 •
edited
Loading

Size Analysis Report 1

This report is too large (78,812 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/Ki4X3MN6V4.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsubox76 hsubox76 hsubox76 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dlarocque @google-oss-bot @hsubox76