-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat(gen ai): add parameter example to evaluation samples #4568
feat(gen ai): add parameter example to evaluation samples #4568
Conversation
@@ -29,6 +29,7 @@ import ( | |||
// evaluateModelResponse evaluates the output of an LLM for groundedness, i.e., how well | |||
// the model response connects with verifiable sources of information | |||
func evaluateModelResponse(w io.Writer, projectID, location string) error { | |||
// location := "us-central1" |
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.
@telpirion , all existing Go samples have this line, but I wonder if it shouldn't be a re-assignment (=
) instead of a fresh assignment (:=
)? i.e.:
// location = "us-central1"
With the current version, if I copy a sample and un-comment this line, I will immediately have no new variables on left side of := ...
compiler error. While with the =
version I'll immediately have a working version of the sample.
What do you think?
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.
Yeah, what you say makes sense. We had written some of the other samples with the thought that users would copy the func, delete the args (or most of them), and then uncomment the variable assignments in the func body.
We haven't received any feedback on this from users (that I'm aware of), so this may be a trivial issue.
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'm fine with the changes to the comments that you recommend. Thank you for checking!
@@ -29,6 +29,7 @@ import ( | |||
// evaluateModelResponse evaluates the output of an LLM for groundedness, i.e., how well | |||
// the model response connects with verifiable sources of information | |||
func evaluateModelResponse(w io.Writer, projectID, location string) error { | |||
// location := "us-central1" |
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.
Yeah, what you say makes sense. We had written some of the other samples with the thought that users would copy the func, delete the args (or most of them), and then uncomment the variable assignments in the func body.
We haven't received any feedback on this from users (that I'm aware of), so this may be a trivial issue.
Description
Adds example value for the
location
argument to all evaluation samples (I missed to add this in the scope of #4514)Checklist
go test -v ./..
gofmt
go vet