Skip to content
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

2271 feedback reward custom metrics #2289

Merged

Conversation

axsaucedo
Copy link
Contributor

PR includes:

  • Extension to support custom metrics in feedback
  • Extra tags to make metrics logged per request unique
  • Simple example using statistical metrics for binary classification

@seldondev
Copy link
Collaborator

Mon Aug 17 13:25:39 UTC 2020
The logs for [lint] [2] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/2.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=2

@seldondev
Copy link
Collaborator

Mon Aug 17 13:25:58 UTC 2020
The logs for [pr-build] [1] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/1.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=1

@seldondev
Copy link
Collaborator

Mon Aug 17 16:10:56 UTC 2020
The logs for [pr-build] [3] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/3.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=3

@seldondev
Copy link
Collaborator

Mon Aug 17 16:11:12 UTC 2020
The logs for [lint] [4] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/4.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=4

@axsaucedo
Copy link
Contributor Author

/test this

@seldondev
Copy link
Collaborator

Mon Aug 17 18:57:12 UTC 2020
The logs for [pr-build] [5] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/5.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=5

@axsaucedo axsaucedo force-pushed the 2271_feedback_reward_custom_metrics branch from e5b9626 to 5f265ef Compare August 17, 2020 18:58
@axsaucedo
Copy link
Contributor Author

Rebasing master

@axsaucedo
Copy link
Contributor Author

/test this
/test lint
/test integration

@seldondev
Copy link
Collaborator

Mon Aug 17 19:00:43 UTC 2020
The logs for [integration] [9] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/9.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=9

@seldondev
Copy link
Collaborator

Mon Aug 17 19:00:43 UTC 2020
The logs for [pr-build] [8] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/8.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=8

@seldondev
Copy link
Collaborator

Mon Aug 17 19:01:00 UTC 2020
The logs for [lint] [10] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/10.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=10

@axsaucedo
Copy link
Contributor Author

axsaucedo commented Aug 18, 2020

Integration tests pass, and python tests pass locally, not sure why lint and pr-build are failing. Lint seems to be failing due to licenses, but pr-build seems to be failing on the operator tests which has nothing to do with pr...

/test this
/test lint

@seldondev
Copy link
Collaborator

Tue Aug 18 05:28:27 UTC 2020
The logs for [lint] [12] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/12.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=12

@seldondev
Copy link
Collaborator

Tue Aug 18 05:28:39 UTC 2020
The logs for [pr-build] [11] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/11.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=11

@axsaucedo
Copy link
Contributor Author

It seems lint test is now fixed and PR build tests are passing except for a flaky test
/test this

@seldondev
Copy link
Collaborator

Tue Aug 18 06:00:35 UTC 2020
The logs for [pr-build] [13] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/13.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=13

@axsaucedo
Copy link
Contributor Author

Seems it's good to go
image

Copy link
Contributor

@jklaise jklaise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Surprised we didn't have an example of this already.

Couple of questions, should we create a documentation page for feedback to go with the example? Also, I see you are using requests to send feedback, does this mean that the seldon client doesn't implement this?

@axsaucedo
Copy link
Contributor Author

Thanks @jklaise! In regards to your first question, the answer is definitely, the next PR will be to add documentation to provide insights on the existing capabilities but also the current discussions (as the existing endpoints will be depricated).

In regards to the second question, that is a very good point, I could've used the feedback function in this situation. In the other example I had to send the ID so it would not have been possible to use the feedback function. Atm the feedback function requires providing a proto message for the request/response so it's a bit of an overhead complexity, but we'll probably want to re-think the python client function as well to make it simpler to use than sending a full request

Copy link
Contributor

@RafalSkolasinski RafalSkolasinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

@@ -80,6 +80,14 @@ def split_image_tag(tag: str) -> Tuple[str]:
DEFAULT_LABELS["image_name"] = DEFAULT_LABELS["model_image"]
DEFAULT_LABELS["image_version"] = DEFAULT_LABELS["model_version"]

FEEDBACK_METRIC_METHOD_TAG = "feedback"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably later on change this list of constants into one enum

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jklaise, RafalSkolasinski

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [RafalSkolasinski,jklaise]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seldondev
Copy link
Collaborator

Wed Aug 19 09:08:40 UTC 2020
The logs for [pr-build] [14] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/14.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=14

@seldondev
Copy link
Collaborator

Wed Aug 19 09:08:54 UTC 2020
The logs for [lint] [15] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/15.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=15

@axsaucedo axsaucedo force-pushed the 2271_feedback_reward_custom_metrics branch from 5f265ef to 6e1e790 Compare August 19, 2020 13:27
@seldondev
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@seldondev seldondev removed the lgtm label Aug 19, 2020
@axsaucedo axsaucedo merged commit a2f3c25 into SeldonIO:master Aug 19, 2020
@seldondev
Copy link
Collaborator

Wed Aug 19 13:32:47 UTC 2020
The logs for [pr-build] [16] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/16.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=16

@seldondev
Copy link
Collaborator

Wed Aug 19 13:34:09 UTC 2020
The logs for [lint] [17] will show after the pipeline context has finished.
https://github.com/SeldonIO/seldon-core/blob/gh-pages/jenkins-x/logs/SeldonIO/seldon-core/PR-2289/17.log

impatient try
jx get build logs SeldonIO/seldon-core/PR-2289 --build=17

@seldondev
Copy link
Collaborator

@axsaucedo: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
lint 6e1e790 link /test lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants