flexflow / FlexFlow

FlexFlow Serve: Low-Latency, High-Performance LLM Serving
https://flexflow.readthedocs.io
Apache License 2.0
1.67k stars 224 forks source link

Doxygen documentation for `lib/substitutions` #1339

Open Bob-Chen222 opened 6 months ago

Bob-Chen222 commented 6 months ago

Description of changes: Created documentation for core functions and structs in the substitution library lib/substitutions. Previous PR closed by accident; created a new one.

For reference, previous PR: #1309

Change made on April 3rd: add tutorial markdown file in the substitution folder

Related Issues:

Linked Issues:

Issues closed by this PR:


This change is Reviewable

lockshaw commented 6 months ago

The high level style here looks pretty good. My only general concern is that some of the docstrings are on the long side--there are many cases in which this is fine (and the explanations generally look pretty good), but remember that all documentation we write now has to be maintained throughout code changes, so while it is important that docstrings include details that help users understand the code beyond the function names and prototypes, it's equally important that they be as short and focused as they can while still being clear. It would be a good idea for you to do a quick pass through and see if there are any parts of the docstrings that you feel aren't critical and can be removed without losing much--not saying there are (as I haven't had time to do a super in-depth read), but it's worth an additional check.

I'm handing this PR off to @wmdi as she's better equipped to do fine-grained checking that everything is correct as she's more familiar with this code. Of course feel free to tag me in if there are any questions on the overall style/structure of the documentation :slightly_smiling_face:

lockshaw commented 4 months ago

Currently waiting on #1394 to get merged, and then needs to be updated with the new substitutions changes there