Skip to content

[docs] Update CPU/GPU inference docs#26881

Merged
stevhliu merged 9 commits into
huggingface:mainfrom
stevhliu:optimize-inference-docs
Oct 31, 2023
Merged

[docs] Update CPU/GPU inference docs#26881
stevhliu merged 9 commits into
huggingface:mainfrom
stevhliu:optimize-inference-docs

Conversation

@stevhliu

Copy link
Copy Markdown
Member

Implements the proposal suggested in #26723, namely:

  • consolidating the inference on one/many GPUs docs into one page
  • removing the inference on specialized hardware page
  • cleanups and updates to the inference docs to provide more context about the how/what of the optimization techniques, code examples so users don't have to skip around to other pages, etc.

@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Oct 20, 2023

Copy link
Copy Markdown

The documentation is not available anymore as the PR was closed or merged.

@MKhalusova MKhalusova left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work on making the inference docs easier to follow. I found a few typos, but otherwise looks great to me.

Comment thread docs/source/en/perf_infer_cpu.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_cpu.md Outdated
@LysandreJik

Copy link
Copy Markdown
Member

@lvwerra did you participate in writing the original document as well? Do you have feedback on this refactor?

@younesbelkada younesbelkada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for updating CPU/GPU inference docs! I left few comments, what do you think?

Comment thread docs/source/en/perf_infer_cpu.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
@regisss

regisss commented Oct 24, 2023

Copy link
Copy Markdown
Contributor

Nice overhaul! I think it's also an opportunity to mention Optimum as this falls right into what we are doing:

WDYT?

Cc @echarlaix @mfuntowicz

@lvwerra

lvwerra commented Oct 24, 2023

Copy link
Copy Markdown
Member

@lvwerra did you participate in writing the original document as well? Do you have feedback on this refactor?

Only did the general structure of that part so leave it to others to comment on content :)

@echarlaix

Copy link
Copy Markdown
Collaborator

Nice overhaul! I think it's also an opportunity to mention Optimum as this falls right into what we are doing:

WDYT?

Cc @echarlaix @mfuntowicz

Totally aligned with your suggestion @regisss, let us know if we can help @stevhliu

@stevhliu

Copy link
Copy Markdown
Member Author

Thanks for the feedback y'all!

Let me know what you think of the new Optimum sections @regisss @echarlaix :)

@regisss regisss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks great, thanks a lot @stevhliu 🙂

Comment thread docs/source/en/perf_infer_cpu.md Outdated

@echarlaix echarlaix left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's really good, thanks a lot @stevhliu 🔥

Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Comment thread docs/source/en/perf_infer_gpu_one.md Outdated
Co-authored-by: regisss <[email protected]>
Co-authored-by: Ella Charlaix <[email protected]>
@stevhliu
stevhliu requested a review from ArthurZucker October 25, 2023 16:59

@LysandreJik LysandreJik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! Only left one comment related to redirection

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this file is deleted, it should redirect to another page with similar information. These pages are linked elsewhere so we should make sure following the previous links don't lead to 404s

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should be fixed, it was only linked in the performance.md doc!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant that it could have been linked in tweets/blogs/things out of our control. I had a feeling we once redirected deleted pages to their updated counterparts; would it be feasible here? Maybe a question to @mishig25 regarding the doc-builder

@regisss regisss Oct 27, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it's possible writing a _redirects.yml (at the same level as _toctree.yml), here is an example in Optimum: https://github.com/huggingface/optimum/blob/main/docs/source/_redirects.yml
It's quite simple, the old link is to the left of the colon and the new one to the right.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's really cool, I didn't know that was possible! I added a _redirects.yml for Transformers, let me know if there is anything else I need to add for it to work 🤗

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks good to me, that should work!

@LysandreJik LysandreJik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for your work @stevhliu!

@stevhliu
stevhliu merged commit 77930f8 into huggingface:main Oct 31, 2023
@stevhliu
stevhliu deleted the optimize-inference-docs branch October 31, 2023 16:44
EduardoPach pushed a commit to EduardoPach/transformers that referenced this pull request Nov 19, 2023
* first draft

* remove non-existent paths

* edits

* feedback

* feedback and optimum

* Apply suggestions from code review

Co-authored-by: regisss <[email protected]>
Co-authored-by: Ella Charlaix <[email protected]>

* redirect to correct doc

* _redirects.yml

---------

Co-authored-by: regisss <[email protected]>
Co-authored-by: Ella Charlaix <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants