Conversation
merge to main branch to avoid conflict
merge to main branch to avoid conflict
Update ll.TextEmbeddingGenerator to 005, update feature, docs, and test. The default vlaue is kept as text-embedding-004 for better customer experience. Bug:381935784
merge to main branch to avoid conflict
…ethod in Gemini-pro-1.5 \n Bug: b/381936588 and b/344891364
…ethod in Gemini-pro-1.5 \n Bug: b/381936588 and b/344891364
GarrettWu
left a comment
There was a problem hiding this comment.
PR title has typos.
It should be adding new features instead of "update".
bigframes/ml/llm.py
Outdated
| """ | ||
| if self._bqml_model.model_name.startswith("gemini-1.5"): | ||
| raise NotImplementedError("Fit is not supported for gemini-1.5 model.") | ||
| # Support gemini-1.5 and gemini-pro |
There was a problem hiding this comment.
move the consts to the top of the file for better organization.
bigframes/ml/llm.py
Outdated
| supported_models = ["gemini-pro", "gemini-1.5-pro-002", "gemini-1.5-flash-002"] | ||
| if self.model_name not in supported_models: | ||
| raise NotImplementedError( | ||
| "Score is not supported models other than gemini-pro or gemini-1.5 model." |
There was a problem hiding this comment.
be more explicit to "gemini-1.5-pro-002" and "gemini-1.5-flash-002".
Since other gemini 1.5 endpoints aren't supported.
bigframes/ml/llm.py
Outdated
| supported_models = ["gemini-pro", "gemini-1.5-pro-002", "gemini-1.5-flash-002"] | ||
| if self.model_name not in supported_models: | ||
| raise NotImplementedError( | ||
| "Score is not supported models other than gemini-pro or gemini-1.5 model." |
There was a problem hiding this comment.
This is in fit() in stead of score().
There was a problem hiding this comment.
I am working on two bugs, b/381936588 and b/344891364, both fit() and score() needed to be updated.
There was a problem hiding this comment.
I mean the message is incorrect
| if self.model_name not in supported_models: | ||
| raise NotImplementedError( | ||
| "Score is not supported models other than gemini-pro or gemini-1.5 model." | ||
| ) |
There was a problem hiding this comment.
I couldn't leave a comment at unchanged lines. Need to update line 905 to each endpoint respectively. (still use gemini-1.0-pro-002 for gemini-pro, but issuing a warning for that case maybe more appropriate)
bigframes/ml/llm.py
Outdated
| ] = "text_generation", | ||
| ) -> bpd.DataFrame: | ||
| """Calculate evaluation metrics of the model. Only "gemini-pro" model is supported for now. | ||
| """Calculate evaluation metrics of the model. Only "gemini-pro" and "gemini-1.5" models are supported for now. |
There was a problem hiding this comment.
same, more explicit to "gemini-1.5-pro-002" and "gemini-1.5-flash-002"
bigframes/ml/llm.py
Outdated
| if self._bqml_model.model_name.startswith("gemini-1.5"): | ||
| raise NotImplementedError("Score is not supported for gemini-1.5 model.") | ||
| # Support gemini-1.5 and gemini-pro | ||
| supported_models = ["gemini-pro", "gemini-1.5-pro-002", "gemini-1.5-flash-002"] |
There was a problem hiding this comment.
Move and share consts to top.
There was a problem hiding this comment.
Not seeing the change? I mean add const variables to the top of the file like https://github.com/googleapis/python-bigquery-dataframes/blob/main/bigframes/ml/llm.py#L67
bigframes/ml/llm.py
Outdated
| supported_models = ["gemini-pro", "gemini-1.5-pro-002", "gemini-1.5-flash-002"] | ||
| if self.model_name not in supported_models: | ||
| raise NotImplementedError( | ||
| "Score is not supported models other than gemini-pro or gemini-1.5 model." |
There was a problem hiding this comment.
Same, more explicit.
Also the sentence seems awkward.
There was a problem hiding this comment.
The message is ill-formed. Maybe just "score() method only supports xxx models".
tests/system/small/ml/test_llm.py
Outdated
| @pytest.mark.flaky(retries=2) | ||
| def test_llm_gemini_pro_score(llm_fine_tune_df_default_index): | ||
| model = llm.GeminiTextGenerator(model_name="gemini-pro") | ||
| # test score() function for "gemini-pro" and "gemini-1.5" model |
There was a problem hiding this comment.
nit: test name already indicates it. The comment seems redundant.
| "gemini-1.5-flash-002", | ||
| ), | ||
| ) | ||
| def test_llm_gemini_score(llm_fine_tune_df_default_index, model_name): |
There was a problem hiding this comment.
add for next test "test_llm_gemini_pro_score_params" as well
This reverts commit 9de2c0e.
bigframes/ml/llm.py
Outdated
| _GEMINI_1P5_PRO_002_ENDPOINT, | ||
| _GEMINI_1P5_FLASH_002_ENDPOINT, | ||
| ) | ||
| _GEMINI_SCORE_ENDPOINTS = ( |
There was a problem hiding this comment.
Can we merge the 2 lists? They are identical, don't think they will diverge.
… since they are identical
…since they are identical" This reverts commit 205e173.
feat: add Gemini-pro-1.5 to GeminiTextGenerator Tuning and Support score() method in Gemini-pro-1.5
Bug: b/381936588 and b/344891364