[BFCL] Fix Double-Casting Issue in model_handler for Java and JS category. #516

HuanzhiMao · 2024-07-08T23:37:57Z

When inferencing for Java or JS category, all function parameters are already in string type (refer to this part). So the model output must be string type as well to be correct.

The decode_ast method in some of the model_handler contains the following code snippet:

if language == "Python":
    pass
else:
    # all values of the json are casted to string for java and javascript
    for key in params:
        params[key] = str(params[key])

The line params[key] = str(params[key]) should not exist because it is giving the model a 'second chance' when its parameter type is wrong (by casting it to a string when it is not). This double type-casting will introduce false positives to the evaluation result. As I mentioned in #477, this PR addresses this issue.

Model Family Affected:

OpenAI (FC only)
Claude (FC only)
Mistral (FC only)
Gemini
Cohere (FC only)

This will affect the leaderboard score. We will update it soon, in a different PR.

Fanjia-Yan

LGMTM

… PR Merge (#536) On July 16th, PR #516 and PR #512 were merged first. They introduced fixes that should be applied to all model handlers. Shortly after, on the same day, PR #525 was merged. The new model handler introduced in PR #525 is missing the fixes from the previous two merged PRs (it wasn't synced accordingly). This PR addresses this issue by applying the necessary fixes to the new model handler.

…est Category (#538) In our BFCL official communication channels, including the evaluation manual blog, GitHub issue replies (such as #424), and our Discord channel, we have previously stated the following: > For Java and JavaScript test category, before querying the model, we do some pre-processing on the prompt and function document. Specifically, at the end of the prompt, we will explicitly state that `the provided function is in Java 8/JavaScript/Python syntax`. And for parameter types that are not native to JSON, we will change their type to `String` (since `String` is JSON compatible) and add in the parameter description that `f" This is Java/JavaScript {value['type']} in string representation."` > As an example, when expecting type `ArrayList`, model will get the instruction that this is a `String` type parameter with the parameter description containing `"This is Java ArrayList in string representation."`, and thus the model should output the value in `String` format (eg, `"new ArrayList<>(Arrays.asList(10, 20, 30))"`), which is JSON compatible. However, the code for `language_specific_pre_processing` did not implement this correctly. Due to an indentation issue, the parameter description was only modified when the parameter type was `any`, and the part where the parameter type is cast to `String` was never implemented. This issue was unnoticed until PR #516 was merged because of the double-casting problem. It *significantly impacts* the evaluation score for the Java and JavaScript categories. We will update the leaderboard very soon. This PR: - Addresses the above issue and ensures that the evaluation logic aligns with the previously described behaviour - Updates two entries in the JavaScript dataset, due to their parameters missing a `description` field - Index: `14, 45` We sincerely apologize for the oversight.

…#539) This PR updates the leaderboard to reflect the changes in score due to the following PR merge. - #516 - #522 - #525 - #536 The score for the following category are affected: - executable_parallel_multiple - Java (FC models only) - JavaScript (FC models only) Consequently, the aggregated also need to be updated: - Simple Func AST - AST Summary - Overall Acc <img width="980" alt="image" src="https://github.com/user-attachments/assets/04ea5265-34ed-4fb4-a2fb-5bc47affa8f7"> <img width="1076" alt="image" src="https://github.com/user-attachments/assets/e37201e1-4cd7-43e8-b37a-5198f4809e87"> --- Aggregates <img width="913" alt="image" src="https://github.com/user-attachments/assets/85b0d681-ffae-42b5-af0d-e7963129bef2"> <img width="809" alt="image" src="https://github.com/user-attachments/assets/04d0de7a-ead7-4e5c-99e4-931864d65e82"> <img width="773" alt="image" src="https://github.com/user-attachments/assets/7df8e885-bd68-4910-896f-a27dcbfefbce">

…gory. (ShishirPatil#516) When inferencing for Java or JS category, all function parameters are already in `string` type (refer to [this part](https://github.com/ShishirPatil/gorilla/blob/main/berkeley-function-call-leaderboard/model_handler/utils.py#L334-L374)). So the model output must be `string` type as well to be correct. The `decode_ast` method in some of the `model_handler` contains the following code snippet: ``` if language == "Python": pass else: # all values of the json are casted to string for java and javascript for key in params: params[key] = str(params[key]) ``` The line `params[key] = str(params[key])` should not exist because it is giving the model a 'second chance' when its parameter type is wrong (by casting it to a `string` when it is not). This double type-casting will introduce false positives to the evaluation result. As I mentioned in ShishirPatil#477, this PR addresses this issue. Model Family Affected: - OpenAI (FC only) - Claude (FC only) - Mistral (FC only) - Gemini - Cohere (FC only) This will affect the leaderboard score. We will update it soon, in a different PR. --------- Co-authored-by: Shishir Patil <30296397+ShishirPatil@users.noreply.github.com>

… PR Merge (ShishirPatil#536) On July 16th, PR ShishirPatil#516 and PR ShishirPatil#512 were merged first. They introduced fixes that should be applied to all model handlers. Shortly after, on the same day, PR ShishirPatil#525 was merged. The new model handler introduced in PR ShishirPatil#525 is missing the fixes from the previous two merged PRs (it wasn't synced accordingly). This PR addresses this issue by applying the necessary fixes to the new model handler.

…est Category (ShishirPatil#538) In our BFCL official communication channels, including the evaluation manual blog, GitHub issue replies (such as ShishirPatil#424), and our Discord channel, we have previously stated the following: > For Java and JavaScript test category, before querying the model, we do some pre-processing on the prompt and function document. Specifically, at the end of the prompt, we will explicitly state that `the provided function is in Java 8/JavaScript/Python syntax`. And for parameter types that are not native to JSON, we will change their type to `String` (since `String` is JSON compatible) and add in the parameter description that `f" This is Java/JavaScript {value['type']} in string representation."` > As an example, when expecting type `ArrayList`, model will get the instruction that this is a `String` type parameter with the parameter description containing `"This is Java ArrayList in string representation."`, and thus the model should output the value in `String` format (eg, `"new ArrayList<>(Arrays.asList(10, 20, 30))"`), which is JSON compatible. However, the code for `language_specific_pre_processing` did not implement this correctly. Due to an indentation issue, the parameter description was only modified when the parameter type was `any`, and the part where the parameter type is cast to `String` was never implemented. This issue was unnoticed until PR ShishirPatil#516 was merged because of the double-casting problem. It *significantly impacts* the evaluation score for the Java and JavaScript categories. We will update the leaderboard very soon. This PR: - Addresses the above issue and ensures that the evaluation logic aligns with the previously described behaviour - Updates two entries in the JavaScript dataset, due to their parameters missing a `description` field - Index: `14, 45` We sincerely apologize for the oversight.

HuanzhiMao added 6 commits July 8, 2024 16:14

update changelog

fb43100

update gpt handler

bb6170d

update claude handler

2c5915c

update gemini handler

ad7909f

update cohere handler, the optimized version is unchanged.

0d41a03

update mistral handler

ac8da73

HuanzhiMao marked this pull request as ready for review July 8, 2024 23:43

Fanjia-Yan approved these changes Jul 11, 2024

View reviewed changes

Merge branch 'main' into double-casting

544bacc

ShishirPatil approved these changes Jul 17, 2024

View reviewed changes

ShishirPatil merged commit 9196826 into ShishirPatil:main Jul 17, 2024

HuanzhiMao deleted the double-casting branch July 17, 2024 06:02

This was referenced Jul 20, 2024

Add Salesforce xLAM handler and fix minor issues #532

Merged

[BFCL] Apply Fix to Newly Introduced Model Handler Missed in Previous PR Merge #536

Merged

This was referenced Jul 21, 2024

[BFCL] Fix language_specific_pre_processing for Java and JavaScript Test Category #538

Merged

[BFCL] Leaderboard Update, in sync with PR #516, #522, #525 and #536. #539

Merged

HuanzhiMao mentioned this pull request Jul 25, 2024

Add Yi Handler #543

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BFCL] Fix Double-Casting Issue in model_handler for Java and JS category. #516

[BFCL] Fix Double-Casting Issue in model_handler for Java and JS category. #516

HuanzhiMao commented Jul 8, 2024

Fanjia-Yan left a comment

[BFCL] Fix Double-Casting Issue in model_handler for Java and JS category. #516

[BFCL] Fix Double-Casting Issue in model_handler for Java and JS category. #516

Conversation

HuanzhiMao commented Jul 8, 2024

Fanjia-Yan left a comment

Choose a reason for hiding this comment