-
Notifications
You must be signed in to change notification settings - Fork 10
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
add --save_all_results_to_dataframe
to benchmark_models cli
#233
add --save_all_results_to_dataframe
to benchmark_models cli
#233
Conversation
… all results to a csv file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % some comments
cli_docs.md
Outdated
@@ -147,7 +147,9 @@ original_request: | |||
╰─────────────────────────┴───────────────────────────────────────────────────────┴──────────────────────────────────┴───────────────╯ | |||
``` | |||
|
|||
You can also filter the completions by tags and generate a report in markdown file using `--file` or `-f`. And run our prompt analyzer (auto-prompt) using `--analyze_prompt`. | |||
You can also filter the completions by tags and generate a report in markdown file using `--file` or `-f`. | |||
If you want to save the all results for post-processing, use `--save_all_results_to_dataframe` to dump it into a csv file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind to add the file name(_all_result_dataframe_dump.csv) which the data will be stored in? I'm also thinking to include that the --file
flag is required for this command.
log10/cli/completions.py
Outdated
@@ -267,7 +267,22 @@ def download_completions(limit, offset, timeout, tags, from_date, to_date, compa | |||
@click.option("--top_p", default=1.0, help="Top p") | |||
@click.option("--analyze_prompt", is_flag=True, help="Run prompt analyzer on the messages.") | |||
@click.option("--file", "-f", help="Specify the filename for the report in markdown format.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you mind to update the help
to include the save results to csv file?
pivot_df["Prompt Messages"] = all_df.groupby("Completion ID")["Prompt Messages"].first() | ||
|
||
# save the dataframe to a csv file | ||
if save_all_results_to_dataframe: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should validate that file
flag is provided when save_all_results_to_dataframe
command is used. So if save_all_results_to_dataframe
is used, but file
isn't provided, we can print out warning message to the users.
7d235de
to
33e1316
Compare
if save_all_results_to_dataframe and not file: | ||
raise click.UsageError("--save_all_results_to_dataframe must be used with --file") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kxtran
check --save_all_results_to_dataframe
and --file
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
to save all results table to a csv file for post-processing.
updated the doc.