-
Notifications
You must be signed in to change notification settings - Fork 570
docs: add tracing notebook #1337
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
Conversation
Documentation preview |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1337 +/- ##
===========================================
+ Coverage 71.59% 71.68% +0.09%
===========================================
Files 168 168
Lines 16862 16862
===========================================
+ Hits 12072 12088 +16
+ Misses 4790 4774 -16
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Attaching a PDF printout of the notebook to make reviewing easy (cc @cparisien , @Pouyanpi , @miyoungc ) |
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.
Thank you @tgasser-nv. This is a valuable addition 👍🏻
Merge plan: Let's ensure this is merged once all pending PRs for 0.16.0 are merged to develop and do a rebase on top of this so that it reflects the latest changes.
Docs build: @miyoungc can you confirm that build_notebook_docs.py is the right path for rendering this notebooks to
https://docs.nvidia.com/nemo/guardrails/latest/getting-started ?
Tutorial usage: right now users can’t follow the docs due to missing configs. Two options, happy with either here:
- Inline the needed configs in the notebook and instruct users to create a config/ folder (like other tutorials), or
- Use
from_contentto avoid file creation entirely.
Nice to haves (follow-ups, separate PR):
- Create NemoGuard NIMs configs to examples/configs so cloned users can reference them directly.
- Use the most recent
span_format: opentelemetry. (I’ll switch the default in #1331; this PR can keep current unless trivial.) The current default format is considered legacy and we might deprecate it. - Show how to toggle parallel/sequential dynamically so we don’t need two folders:
config.rails.input.parallel = True
config.rails.output.parallel = False
-
point users to the “Parallel Execution of Input and Output Rails” section.
docs: https://docs.nvidia.com/nemo/guardrails/latest/user-guides/configuration-guide/guardrails-configuration.html#parallel-rails -
quiet installs to keep notebook output clean:
!pip install pandas plotly langchain_nvidia_ai_endpoints aiofiles -q
If we pick one of the two config approaches above and rebase after 0.16.0 lands it'd be great.
cparisien
left a comment
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 looking really good! Needs a little bit of cleanup but it's in pretty good shape already.
5a46bb5 to
9dd80a4
Compare
Quick update:
I met with @miyoungc who's looking at the best way to link the notebook from the rest of the docs. Am holding off merging until Monday to give her time to work on this. |
* doc edits for tracing notebook * more improvs * last bits of editing
Changes links to docs from relative to absolute Co-authored-by: Miyoung Choi <miyoungc@nvidia.com> Signed-off-by: Tim Gasser <200644301+tgasser-nv@users.noreply.github.com>
Pouyanpi
left a comment
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.
Thank you Tim! It looks good to me. Just a very minor issue that I commented and 2 questions. Please merge it when you are ready 👍🏻
* Initial checkin of first tracing notebook * Removed unused files * Finish notebook * Remove config files, these are created programmatically now * Rewrote notebook to remove config files, cleared output cells * Update notebook * Clean run with latest develop branch * Remove unsafe prompts from notebook * Added link to Parallel Rails docs * Run black linter over notebook * docs: doc edits for tracing notebook (#1349) * doc edits for tracing notebook * more improvs * last bits of editing * Add Miyoung's updates * Apply miyoung's suggested changes Changes links to docs from relative to absolute Co-authored-by: Miyoung Choi <miyoungc@nvidia.com> Signed-off-by: Tim Gasser <200644301+tgasser-nv@users.noreply.github.com> * Address Pouyan's comments --------- Signed-off-by: Tim Gasser <200644301+tgasser-nv@users.noreply.github.com> Co-authored-by: Miyoung Choi <miyoungc@nvidia.com>
* Initial checkin of first tracing notebook * Removed unused files * Finish notebook * Remove config files, these are created programmatically now * Rewrote notebook to remove config files, cleared output cells * Update notebook * Clean run with latest develop branch * Remove unsafe prompts from notebook * Added link to Parallel Rails docs * Run black linter over notebook * docs: doc edits for tracing notebook (#1349) * doc edits for tracing notebook * more improvs * last bits of editing * Add Miyoung's updates * Apply miyoung's suggested changes Changes links to docs from relative to absolute Co-authored-by: Miyoung Choi <miyoungc@nvidia.com> Signed-off-by: Tim Gasser <200644301+tgasser-nv@users.noreply.github.com> * Address Pouyan's comments --------- Signed-off-by: Tim Gasser <200644301+tgasser-nv@users.noreply.github.com> Co-authored-by: Miyoung Choi <miyoungc@nvidia.com>
Description
Related Issue(s)
Checklist