-
Notifications
You must be signed in to change notification settings - Fork 18
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: quarto monitoring dashboard template #203
Conversation
monitoring_dashboard.qmd
Outdated
``` | ||
|
||
```{python} | ||
## the next few lines are an example model, here is a place to |
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.
This is maybe too many lines of code for a template, but would mtcars be something too simple?
.github/workflows/tests.yml
Outdated
@@ -159,3 +159,25 @@ jobs: | |||
- name: Run Tests | |||
shell: bash | |||
run: make typecheck | |||
|
|||
run-templates: |
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.
This will try to render each template as a "test" to make sure they work out of the box.
@@ -0,0 +1,42 @@ | |||
from importlib_resources import files as _files |
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.
You could previously vetiver.model_card.model_card
OR vetiver.model_card
. The first option is no longer available, it is now vetiver.templates.model_card
or you can still vetiver.model_card
. I doubt anyone is using vetiver.model_card.model_card
, so I feel pretty okay making this swap.
``` | ||
|
||
```{python} | ||
## the next few lines are an example model, here is a place to |
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.
This might be a little heavyweight to just load in monitoring data, but I wonder if something like mtcars is too simple?
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 do think something like mtcars is too simple because it doesn't have dates. This seems pretty reasonable, although some kind of built-in data would be great too.
monitoring_dashboard.qmd
Outdated
inspections_new["results"] = inspections_new["results"].map({"PASS": 0, "FAIL": 1}) | ||
``` | ||
|
||
# Model card |
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 hadn't really thought about putting the Model Card content in the monitoring dashboard template itself (vs. the example for the Quarto gallery). On the one hand, it's a good way to surface this and I suspect people may in fact find this a good way to go (one piece of published content with model info). On the other hand, this code is now in two places and we'd have to keep this updated in both, and this template now takes more work to get customized and is not just about monitoring.
What are your thoughts on this? Do you have a strong sense that we should include the Model Card info within the monitoring template?
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.
Hmm...maybe we don't have a heading that says # Model Card
, but just include some of the information? It does seem nice to have a little bit of context, but it is a valid point that people don't want to maintain this information (or that we don't want to maintain this code 😂) in two locations.
What do you think about keeping the automatable pieces that are useful for context/people won't have to customize, and then pull pretty much everything else? Concretely, we could keep:
v.description
v.model_name
- Printing the input prototype
- Prompting for "The model was created by ..."
as the index page of the dashboard.
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's a good way to go. 👍
monitoring_dashboard.qmd
Outdated
@@ -0,0 +1,138 @@ | |||
--- | |||
title: "Monitoring dashboard" |
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.
Is this file in two places, in the root of the repo as well as in templates/
? Is that right?
monitoring_dashboard.qmd
Outdated
|
||
- We recommend ... | ||
|
||
# Explore validation data |
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.
# Explore validation data | |
# Model metrics |
Or "Model performance" or similar. I think "Explore validation data" implies more, say, EDA of the validation data vs. connecting back to the model.
Showing an example of that (just a histogram for the validation data) is included in the R template, and I do think that's a useful category of thing to show people (not metrics, just what the new data is like). Thoughts?
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.
Ah, sure! That makes sense to encourage more exploration of incoming data. I'll add that in.
``` | ||
|
||
```{python} | ||
## the next few lines are an example model, here is a place to |
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 do think something like mtcars is too simple because it doesn't have dates. This seems pretty reasonable, although some kind of built-in data would be great too.
Adds a monitoring dashboard template for a VetiverModel object/deployment.