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

Inference Documentation #3119

Merged
merged 14 commits into from
Jun 12, 2023
Merged

Inference Documentation #3119

merged 14 commits into from
Jun 12, 2023

Conversation

alando46
Copy link
Contributor

This is a mostly done (although not totally complete) PR with a technical overview of the inference architecture. I'm looking forward to high level feedback (general layout, flow of documentation) or specific suggestions (I'm sure I made some errors or missed some details.) I will try to wrap up the final section soon.

See related discussion on the issue: #1473 (comment)

@github-actions
Copy link

pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md

@GuilleHoardings
Copy link
Contributor

Nice work!

It looks like you went for a quite detailed explanation of the code. As a suggestion, I think that a diagram of how all the components are connected, and a brief description of each one's goal, would help.

Also, I think it's important to try to use always the same word for the different components. A couple of examples of things I've seen which I think they are the same, but I'm not sure:

  • In the architecture document, it talks about a text client, that it's referred to as Text Client, text-client, and a REPL client in the inference document.

  • In the architecture document, it talks about a FastAPI webserver, that it's referred to as OA server in the mermaid diagram and OA Inference Server in the inference document.

Anyway, these are minor things. Thank you again for the work!

@yk
Copy link
Collaborator

yk commented May 11, 2023

thanks a lot for this :)

@github-actions
Copy link

pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md

@andreaskoepf
Copy link
Collaborator

@alando46 this PR is currently still in draft status. Is it ready for review?

@alando46
Copy link
Contributor Author

alando46 commented Jun 12, 2023

@andreaskoepf, would it be possible to raise this as-is (w/o last section)? I keep trying to get back to this but things have gotten unexpectedly busy.

I'd propose raising this as-is, and then down the road I or someone else can update the final section. The plugin addition (very cool) will likely change some things on the server/worker inference system so might make sense to update the inference documentation once again anyways once that api stabilizes a bit.

Thoughts?

@olliestanley olliestanley linked an issue Jun 12, 2023 that may be closed by this pull request
@olliestanley olliestanley marked this pull request as ready for review June 12, 2023 17:54
@olliestanley olliestanley enabled auto-merge (squash) June 12, 2023 17:54
@olliestanley olliestanley merged commit 470fbe9 into LAION-AI:main Jun 12, 2023
2 checks passed
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.

Document inference pipeline
5 participants