Skip to content
This repository has been archived by the owner on Aug 10, 2024. It is now read-only.

Simplify get started - condensed #238

Merged
merged 5 commits into from
Dec 5, 2022
Merged

Conversation

omesser
Copy link
Contributor

@omesser omesser commented Nov 26, 2022

https://mlem-ai-simplify-get-st-rnmia5.herokuapp.com/doc/get-started

Here, I'll create an alternative where there is a single get-started scenario, but will use some alternative paths in this scenarios (so the user can choose).

Details:

  • used tabs for alternative model prediction (python, cli)
  • kept local model serving as a distinct stage (because it's "basic" and simple)
  • used tabs for alternative production deployments (docker, heroku) - we can add kubernetes here IMO. But I will wait with this until a more high level discussion about the structure is done

More context: #228 (comment)

@omesser omesser marked this pull request as draft November 26, 2022 17:23
@shcheklein shcheklein temporarily deployed to mlem-ai-simplify-get-st-rnmia5 November 26, 2022 17:23 Inactive
@omesser
Copy link
Contributor Author

omesser commented Nov 26, 2022

Hi @iterative/websites ! I would love your assistance in preparing this suggestion / use case
I would like to experiment with introducing alternative use-cases to the get-started scenario of mlem, similar to the "CML Use Cases" in cml.dev. Here, there's no need for a switcher (e.g. gh,gl,bb) just a one-level tab system which I would hopefully be able to embed and reuse 2-3 times in the get-started page.
It's totally fine if I would have to break the page down to multiple markdown pages or edit some ts code or similar (but would love some guidance here).
Currently the docs are "pure markdown" - meaning, I don't see an easy way for me to introduce this without changing the site's structure in addition to bringing-over/creating a tab component similar to JSONTabs in cmldev.

Can you please help me out with this - provide me some simple tab structure/component in this branch I can then use?
Would appreciate the help 🙏

@github-actions
Copy link

github-actions bot commented Nov 26, 2022

e390226

Link Check Report

There were no links to check!

@aguschin
Copy link
Contributor

aguschin commented Nov 27, 2022

@omesser, if you would like to make something like this (from https://cml.dev/doc/cml-with-dvc):
image

Then you need to use something like

<toggle>
<tab title="GitHub">
...
</tab>
<tab title="GitHub">
...
</tab>
</toggle>

see this file as an example https://raw.githubusercontent.com/iterative/cml.dev/master/content/docs/cml-with-dvc.md

Curious to see how it will look like)

@yathomasi
Copy link
Contributor

In addition to @aguschin suggestion of using tabs, I think we can also use <details> tag if tabs don't make sense.
Example: https://dvc.org/doc/user-guide/overview#comparison-with-related-technologies
Usage:

<details>

### Title

...
content
...
</details>

@omesser
Copy link
Contributor Author

omesser commented Nov 28, 2022

@omesser, if you would like to make something like this (from https://cml.dev/doc/cml-with-dvc): image

Then you need to use something like

<toggle>
<tab title="GitHub">
...
</tab>
<tab title="GitHub">
...
</tab>
</toggle>

see this file as an example https://raw.githubusercontent.com/iterative/cml.dev/master/content/docs/cml-with-dvc.md

Curious to see how it will look like)

Thanks @aguschin - that toggle/tab system is exactly what I was looking for 🙏

In addition to @aguschin suggestion of using tabs, I think we can also use <details> tag if tabs don't make sense. Example: https://dvc.org/doc/user-guide/overview#comparison-with-related-technologies Usage:

<details>

### Title

...
content
...
</details>

Re<details> - thanks @yathomasi we're already using this quite extensively, was looking for tabs specifically!

@omesser omesser force-pushed the simplify_get_started_condensed branch from fa35c9a to 02c7687 Compare November 28, 2022 21:25
@omesser omesser temporarily deployed to mlem-ai-simplify-get-st-rnmia5 November 28, 2022 21:26 Inactive
@omesser omesser force-pushed the simplify_get_started_condensed branch from 02c7687 to 6788c6e Compare November 29, 2022 22:45
@omesser omesser temporarily deployed to mlem-ai-simplify-get-st-rnmia5 November 29, 2022 22:46 Inactive
@omesser omesser marked this pull request as ready for review November 29, 2022 22:50
@omesser omesser marked this pull request as draft November 29, 2022 22:50
@omesser
Copy link
Contributor Author

omesser commented Nov 29, 2022

@aguschin @mike0sv @jorgeorpinel - please take a look and compare this with

Would like to get your thoughts

@omesser omesser changed the title Simplify get started - condensed - WIP Simplify get started - condensed Nov 29, 2022
@mike0sv
Copy link
Contributor

mike0sv commented Nov 30, 2022

For me this feels like a lot of text for one page 😄 I mean that I'd probably cut a lot of extra explanations and leave just something like this "Here's how to do X: <toggle>different approaches</toggle>. It just works. Next, how to do Y...". I'll try to make a draft like this just to see how it looks

U: see #244

@omesser omesser force-pushed the simplify_get_started_condensed branch from 6788c6e to 1132e19 Compare November 30, 2022 15:48
@omesser omesser temporarily deployed to mlem-ai-simplify-get-st-rnmia5 November 30, 2022 15:52 Inactive
@aguschin
Copy link
Contributor

aguschin commented Dec 1, 2022

Kinda agree with @mike0sv - the page is long and I was thinking to hide serve/build/deploy behind a single toggle (probably with deploy as page that we show by default).

But this reminds me of discussion we had in #190 (comment), TLDR, there I suggested something like:

I have [ CV | Table data | NLP ] task
...<the selected part is shown>...

I want to [ serve | build | deploy ] my model
...<the selected part is shown>...

And this ^ is a configuration page to my mind, not a GS. So if we unite serve/build/deploy behind a single toggle, I'd like not to get this configuration page, but get a Get Started.

As a suggestion: we can keep in mind that we'll have this configuration page in some future. How to write GS so it would complement, but not replace that page?

@mike0sv
Copy link
Contributor

mike0sv commented Dec 1, 2022

Have a look at #244 (env)
It's a lot messier that this but it showcases my point I guess

@aguschin
Copy link
Contributor

aguschin commented Dec 2, 2022

I would like to experiment with introducing alternative use-cases to the get-started scenario of mlem, similar to the "CML Use Cases" in cml.dev. Here, there's no need for a switcher (e.g. gh,gl,bb) just a one-level tab system which I would hopefully be able to embed and reuse 2-3 times in the get-started page.

@yathomasi, is there a way to achieve something similar? Like nested toggles?

Tried this and it doesn't work:

<toggle>
<tab title="TAB">

tab content

<toggle>
<tab title="SUBTAB">

subtab content

</tab>
</toggle>

</tab>
</toggle>

@yathomasi
Copy link
Contributor

yathomasi commented Dec 2, 2022

@yathomasi, is there a way to achieve something similar? Like nested toggles?

It is available @aguschin. We have already started using nested tabs eg: https://dvc.org/doc/install/completion#what-shell-do-you-have. Also, the syntax is correct.

It looks like there is an issue with mlem.ai I will take a look.

@yathomasi
Copy link
Contributor

I just checked with the exact code you have provided on the current branch and it seems to be working.
Can you try yarn clean and run it again? cc: @aguschin

Screenshot 2022-12-02 at 19 08 15

@aguschin aguschin marked this pull request as ready for review December 5, 2022 11:15
Copy link
Contributor

@aguschin aguschin left a comment

Choose a reason for hiding this comment

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

After a discussion with @mike0sv and @omesser we decided:

  1. select this variant of GS instead of others (current with apply/build/serve/deploy; Get Started updates #228; Simplify get started very condenced #244)
  2. @omesser will make another PR on top of this taking some changes from Simplify get started very condenced #244

@aguschin aguschin merged commit e390226 into main Dec 5, 2022
@aguschin aguschin deleted the simplify_get_started_condensed branch December 5, 2022 11:18
@jorgeorpinel jorgeorpinel added A: docs Area: user documentation (gatsby-theme-iterative) type: enhancement Something is not clear, small updates, improvement suggestions C: get-started Content of /doc/get-started labels Dec 8, 2022
@jorgeorpinel jorgeorpinel mentioned this pull request Dec 8, 2022
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Minor? I would not use the term "tutorial". We don't really have any proper tutorials here (maybe in the company blog though). Tutorial: walkthrough solving specific/real-life problem.

content/docs/command-reference/serve.md Show resolved Hide resolved
content/docs/get-started/index.md Show resolved Hide resolved
content/docs/user-guide/building/index.md Show resolved Hide resolved
@jorgeorpinel jorgeorpinel removed the type: enhancement Something is not clear, small updates, improvement suggestions label Dec 9, 2022
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Dec 10, 2022

Sorry, late review but I hope still relevant since there seems to be a plan to follow up on this per

@omesser will make another PR on top of this taking some changes from #244

I turned #244 into a draft for now, BTW.


feels like a lot of text for one page...
hide serve/build/deploy behind a single toggle

Yep, not sure why this is called condensed when it adds back several things we had previously consciously removed (e.g. mlem serve and build), doubling the overall length. We also introduced significant more structure and the section titles are confusing (Deployment, then Serve, then Deploy again? But it's actually about building/packaging...). The GS should be straightforward. Again, we had already decided to leave some things out for this reason. I think we should be a bit more careful with GS changes (OK to discuss for some time) to avoid going in circles.

a way to achieve something similar? Like nested toggles?

Keep in mind this requires a lot of clicking so it's not ideal for this doc. Not even sure about the plain tabs TBH (GS should be linear ideally) -- but I guess it can be fine as long as the "happy path" is always visible in the first tabs.

@shcheklein
Copy link
Member

@jorgeorpinel in this case tabs should work better I think (similar to GitHub/GitLab) in CML. MLEM is not DVC. Get started doesn't have to be same and/or complicated as in DVC. It is condensed since it's a single page vs multiple, less clicks and text overall I hope. Other things - yes, we should be clarifying and reviewing titles if that makes sense). Write your specific concerns somewhere please.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Dec 12, 2022

I have lots of several specific concerns but since this review is late I decided to do a PR instead (didn't push it yet) ⏳

jorgeorpinel added a commit that referenced this pull request Dec 13, 2022
and move some info to the UG index page

rel. #238 (comment)
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

Some more comments (questions with check boxes cc @aguschin @omesser) I'll addressi n #265 as well. Thanks

content/docs/get-started/index.md Show resolved Hide resolved

<tab title="Command Line">

### Batch scoring
Copy link
Contributor

@jorgeorpinel jorgeorpinel Dec 13, 2022

Choose a reason for hiding this comment

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

Not seeing a batch operation here..? Is there a better term than "batch" here?

Copy link
Contributor Author

@omesser omesser Dec 25, 2022

Choose a reason for hiding this comment

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

this is indeed batch scoring (using the model to infer on a concrete dataset). Maybe the confusion is because the "batchness" is not demonstrated clearly. the example uses a "batch" of a single vector, but the csv (or whatever format) file here can be of arbitrary size (multiple rows in the csv) and the apply command will run through the entire datasets provided and infer the model with it.

@aguschin - Maybe the example should be slightly modified to emphasize the above - just echoing another line should do it :D

Copy link
Contributor

@aguschin aguschin Dec 26, 2022

Choose a reason for hiding this comment

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

@jorgeorpinel will it fix your confusion? Or maybe you refered to something like --batch_size X param that will process data in chunks of len X? Like:

$ mlem apply models/rf new_data.csv \
    --method predict_proba \
    --import \
    --import-type "pandas[csv]" \
    --batch_size 1

Copy link
Contributor

@jorgeorpinel jorgeorpinel Dec 27, 2022

Choose a reason for hiding this comment

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

the example uses a "batch" of a single vector, but the csv... can be of arbitrary size

--batch_size 1

Can we make the example more realistic, so that the batch is actually a batch (a group of things) ? 🙂

But I'm getting the feeling that the batchness aspect is not important here, which agains brings me to suggest we rename this to something like Offline model scoring (Idk, need your input to find a good title). Otherwise term "batch" may be a distraction (in the title).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's still very much a toy example (that's fine for the get-started imo) - but took a stab at it here - simple making the dataset not a single vector but 2 should get the idea across imo

Copy link
Contributor

Choose a reason for hiding this comment

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

OK from this I get that it is important to stress the batchness. Having 2 lines instead of 1 sounds simple and good. Thanks

Comment on lines +293 to +299
## Local model serving

First thing first, let's run a model server locally on our machine. To launch a
local FastAPI model server, simply run:

```cli
$ mlem serve fastapi --model models/rf
Copy link
Contributor

@jorgeorpinel jorgeorpinel Dec 13, 2022

Choose a reason for hiding this comment

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

I thought we had decided to leave this out of the GS? Focus on deployment (which is more relevant and includes serving).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not aware of this - it depends how aggressively we want to cut length here. @aguschin - thoughts?

Copy link
Contributor

@aguschin aguschin Dec 26, 2022

Choose a reason for hiding this comment

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

I bet almost every user would run mlem serve multiple times for his model until he decides to do mlem deploy. So it's good that we introduce it here.

Also, off topic - maybe we need to mlem serve streamlit (see UG) in GS? It will have a nicer look I assume. (from the other hand streamlit is not that polished now, and we use it in every blog post anyway - so keeping fastapi for now may be a better option).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, we should keep it then.

maybe we need to mlem serve streamlit

Up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think fastapi works better for us - better adoption and swagger api spec are a great place to showcase a "by the book" RESTapi server

example FastAPI or RabbitMQ. Here we'll check out how it works with FastAPI
since serving models via a REST API is a very common use case.

## Local model serving
Copy link
Contributor

@jorgeorpinel jorgeorpinel Dec 13, 2022

Choose a reason for hiding this comment

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

Confusing structure... (see this section and its sub-sections)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I get what you mean here exactly - I totally see a room for confusion between serving and deploying. I think it might be a real confusion in MLEM's UX we need to think how to improve.
Is that what you mean? or something else? can you elaborate a bit?

Copy link
Contributor

@jorgeorpinel jorgeorpinel Dec 27, 2022

Choose a reason for hiding this comment

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

I've started addressing this in a follow-up PR. Here's some more context:#265 (review)

room for confusion between serving and deploying

Exactly. I'm not sure yet if it's something docs-specific or more of a product question (UX) indeed. But surely we can at least ameliorate the issue with good docs structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's a real product UX question - cc @aguschin @mike0sv let's discuss this a bit, brainstorm if we can simplify or communicate this differently - I get the confusion @jorgeorpinel has here 💯 .

For now I've built this and the next proposed iteration of get-started taking the current abstraction/structure as given (deployment builds on top serving) since the actual code is structured around this concept

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, thanks. We may have conflicts among PRs though... I'll focus on finishing mine first.

content/docs/get-started/index.md Show resolved Hide resolved
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Dec 14, 2022

Please see #265 for some more concerns and follow-ups.

jorgeorpinel added a commit that referenced this pull request Dec 27, 2022
jorgeorpinel added a commit that referenced this pull request Dec 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A: docs Area: user documentation (gatsby-theme-iterative) C: get-started Content of /doc/get-started
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants