langchain-google-vertexai: Update vision_models.py to allow kwargs usage

[TommasoPetrolito]

Checklist for PR Creation

• PR Title: "langchain-google-vertexai: Update vision_models.py to allow kwargs usage"

• PR Description and Relevant issues:

  • This change touches _prepare_params() methods in order to avoid that keys not included in default parameters are filtered out systematically

def _prepare_params(self, **kwargs: Any) -> Dict[str, Any]:
        params = self._default_params
        for key, value in kwargs.items():
             if key in params and value is not None:
                    params[key] = value

changed to:

def _prepare_params(self, **kwargs: Any) -> Dict[str, Any]:
        params = self._default_params
        for key, value in kwargs.items():
             if value is not None:
                   params[key] = value

• This change also touches all the final calls to vertexai image related functions when they take into account the additional parameters, they were only taking into account, for some reason, messages[0].additional_kwargs, instead kwargs coming from outer functions calls were not taken into account and have been added:

        image_str_list = self._generate_images(
            prompt=user_query, **messages[0].additional_kwargs
        )

changed to:

        image_str_list = self._generate_images(
            prompt=user_query, **messages[0].additional_kwargs, **kwargs
        )

• Relevant issues: It was not possible to use kwargs like {"safety_filter_level": "block_few", "person_generation": "allow_all"} from langchain-core BaseChatModel invoke() method. They got filtered out and never reached the final image generation call.

• No specific dependencies required for this fix.

• Add Tests and Docs:

  • None

• Lint and Test:

  • The amount of changes is limited to very specific points and should not require lint check, as you can see
  • I have tested it by directly pip installing from this commit hash

PR Description

This PR addresses the impossibility to use additional kwargs with models classes' methods inside langchain_google_vertexai/vision_models.py. E.g.: It was not possible to pass parameters like {"safety_filter_level": "block_few", "person_generation": "allow_all"} to the image generation, these parameters used to get filtered out systematically.

Relevant issues

Type

🐛 Bug Fix

[TommasoPetrolito]

Alternatively, instead of disabling the filtering out of non-default keys contained inside self._default_params, all the useful parameters could be added to default_params accordingly.
Of course this alternative approach would require continuous timely update of the list of default params.
Both options are OK and would allow library users to effectively use kwargs.

[lkuligin]

Thanks for your contribution!
I'm not fully sure it's an ideal way of doing things. An alternative would be to add additional keys to default_params with None values (to be tested, but typically protos on the server side handle None values well). Since after this change if a user includes into kwargs an argument that is not supported by the proto, the model will fail (since this argument will be always passed further).

@efriis @baskaryan What is the general approach, should we be graceful to any argument in **kwargs that is not accepted by the underlying model, or should we fail? Maybe I'm overthinking here.

[TommasoPetrolito]

Thanks for your contribution!
I'm not fully sure it's an ideal way of doing things. An alternative would be to add additional keys to default_params with None values (to be tested, but typically protos on the server side handle None values well). Since after this change if a user includes into kwargs an argument that is not supported by the proto, the model will fail (since this argument will be always passed further).

@efriis @baskaryan What is the general approach, should we be graceful to any argument in **kwargs that is not accepted by the underlying model, or should we fail? Maybe I'm overthinking here.

Thank you very much for your feedback!
I totally agree, I am not sure as well.
I elaborate a bit more what my thought / reasoning was while approaching this issue.
For sure, not being able to use parameters that are actually supported by the final model is not good.
I have imagined two possible alternative approaches:

• Enabling complete kwargs usage free of constraints and allowing the outer code to pass any key:value pair. It would be responsibility of the library user alone to put-in the correct keys and values that are actually existing and making sense
• Adding all parameters that are currently actually supported by the final vertexai code and models as key:value pairs inside default_params to avoid them to be wrongly and disturbingly filtered out

While reading at the code my thought was "it depends on what is the exact purpose of prepare_params and default_params", I might have interpreted them wrong.
If they are there to automatically fill the default/core params with default values that are only eventually replaced by kwargs values with matching keys, then filtering out additional non-default params/keys might be an undesired behavior, maybe just a leftover.
If default_params is more like a supported_params, then yes, it makes more sense to add all the supported parameters' keys to default_params.

To give an idea of the amount of currently unhandled parameters, that are filtered out with current code version, the following are the parameters actually supported by vertexai ImageGenerationModel's _generate_images() method in vertexai/vision_models/_vision_models.py, as per today 2024/September/06 (checked ones are the ones already included inside default_params):

• negative_prompt: Optional[str] = None,
• number_of_images: int = 1,
• width: Optional[int] = None,
• height: Optional[int] = None,
• aspect_ratio: Optional[Literal["1:1", "9:16", "16:9", "4:3", "3:4"]] = None,
• guidance_scale: Optional[float] = None,
• seed: Optional[int] = None,
• base_image: Optional["Image"] = None,
• mask: Optional["Image"] = None,
• edit_mode: Optional[
  Literal[
  "inpainting-insert",
  "inpainting-remove",
  "outpainting",
  "product-image",
  ]
  ] = None,
• mask_mode: Optional[Literal["background", "foreground", "semantic"]] = None,
• segmentation_classes: Optional[List[str]] = None,
• mask_dilation: Optional[float] = None,
• product_position: Optional[Literal["fixed", "reposition"]] = None,
• output_mime_type: Optional[Literal["image/png", "image/jpeg"]] = None,
• compression_quality: Optional[float] = None,
• language: Optional[str] = None,
• output_gcs_uri: Optional[str] = None,
• add_watermark: Optional[bool] = None,
• safety_filter_level: Optional[
  Literal["block_most", "block_some", "block_few", "block_fewest"]
  ] = None,
• person_generation: Optional[
  Literal["dont_allow", "allow_adult", "allow_all"]
  ] = None,
  )

I think any solution allowing to use at least all parameters currently supported by methods defined inside _vision_models.py (also simply adding them to default_params with None default value is perfect) would work fine from a library-user POV, anything is better then just having the additional params just silently ignored (even a crash or error of the model would be better so that the user is at least aware of whether or not they are taken into account).

From the mantaining POV, adding all supported params to default_params means, possibly, editing this code every time any method/model inside vertexai library _vision_models.py (captioning, image generation, image editing, image Q&A, etc.) starts supporting and handling new parameters or drops a parameter, while accepting all of them without filtering them out, only puts the responsibility of passing actually existing parameters on the user side.

[lkuligin]

@TommasoPetrolito feel free to take a look how it works here, please:

langchain-google/libs/vertexai/langchain_google_vertexai/chat_models.py

Line 605 in 8958221

params = (

maybe we could align the approach, wdyt?

[TommasoPetrolito]

@TommasoPetrolito feel free to take a look how it works here, please:

langchain-google/libs/vertexai/langchain_google_vertexai/chat_models.py

Line 605 in 8958221

params = (

maybe we could align the approach, wdyt?

Hello @lkuligin, thank you again for your feedback, it is not 100% clear to me what you mean here. Do you suggest to proceed on a code change approach involving checks on the supported parameters that imitates / is coherently aligned with the code as done here:

langchain-google/libs/vertexai/langchain_google_vertexai/chat_models.py

Line 605 in 8958221

params = (

?

Or you mean that we should discuss the approach a bit further here and align our views on the approach after a discussion?

I can, for sure, change my code proposal inside vision_models.py to include a supported_params specific variable/property for each vision model (listing all supported parameters already handled by vertexai library vision models) in the same way as done here:

langchain-google/libs/vertexai/langchain_google_vertexai/chat_models.py

Line 605 in 8958221

params = (

and implement coherently a check with filtering based on those variable/property instead of default_params, that would then be used only for the automatic filling-in of default values for those default_params key=value pairs, not to filter out params.

What do you think?

Thank you again.