-
Notifications
You must be signed in to change notification settings - Fork 62
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 methods to log the results of the api runner to a server #177
Changes from 6 commits
5ff0832
f1d2bc6
4877c5f
da54167
5c93586
2f74233
f4a7484
879ded4
566081b
ef3ab5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ | |
parser.add_argument("-v", "--verbose", action="store_true") | ||
parser.add_argument("-l", "--logprobs", action="store_true") | ||
parser.add_argument("--upload_url", type=str) | ||
parser.add_argument("--run_name", type=str, required=False) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added a param to optionally add a run name when we are running an eval this run name is then stored as a static JSON object, and can be used by the eval visualizer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! nit: shall we update the README with the new option as well? 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, thank you for pointing this out! |
||
parser.add_argument( | ||
"-qz", "--quantized", default=False, action=argparse.BooleanOptionalAction | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# this is a Google cloud function for receiving the data from the web app and storing it in the database | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: shall we add the command for launching this cloud function (in case we need to modify it and relaunch in the future)? |
||
|
||
import functions_framework | ||
from google.cloud import storage | ||
import json | ||
|
||
BUCKET_NAME = "YOUR-BUCKET-NAME" | ||
|
||
|
||
@functions_framework.http | ||
def hello_http(request): | ||
request_json = request.get_json(silent=True) | ||
results = request_json["results"] | ||
run_name = request_json["run_name"] | ||
storage_client = storage.Client() | ||
bucket = storage_client.bucket(BUCKET_NAME) | ||
blob = bucket.blob(run_name + ".json") | ||
blob.upload_from_string(json.dumps(results)) | ||
return "success" |
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.
it seems like
results
is only created in L289 whenlogprobs
is true:And I think this might fail if
logprobs
is True andupload_url
is provided (eg if the user is not exporting logprobs, say when using TGI or some other inference API), sinceresults
hasn't yet been defined in that code path.Shall we update the code before the check in L285 to output
results
regardless of whetherlogprobs
is provided?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.
Great point, done!