-
Notifications
You must be signed in to change notification settings - Fork 516
docs: add parallel rail section + split config page #1295
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 #1295 +/- ##
===========================================
+ Coverage 70.13% 70.45% +0.31%
===========================================
Files 161 161
Lines 16037 16214 +177
===========================================
+ Hits 11248 11423 +175
- Misses 4789 4791 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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 @miyoungc 👍🏻 , I did a quick iteration and I like the new structure. I'll do another in depth round soon.
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.
Summary
I only reviewed the parallel rails and RAG/knowledge base sections. I'm assuming the rest is the splitting of configuration into multiple files.
The Parallel Rails section needs updating, here's some more context on the feature.
For true parallel rail calling for any configuration, we need to use Colang 2. This isn't yet integrated and tested in Guardrails. It would take ~2 months work to get Colang 2 up to feature parity with Colang 1, so we decided to point-fix Colang 1 to enable parallel rail execution.
Because this is a point-fix and will be deprecated when Colang 2 is adopted, we limited the scope to support two templates:
* https://github.com/NVIDIA/NeMo-Guardrails/blob/develop/examples/notebooks/safeguard_ai_virtual_assistant_notebook.ipynb
* https://developer.nvidia.com/blog/content-moderation-and-safety-checks-with-nvidia-nemo-guardrails/
These two templates are QA'ed and validated to work. Other configs may work, but we haven't tested them and users are encouraged to validate them themselves. Configs which mutate input data aren't guaranteed to give the same output as sequential rails, since they're executing in parallel with no dependency tracking.
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
Yes, that's correct. The parallel rails section in |
@tgasser-nv Added the example configurations to the same section for parallel rail, instead of creating a new tutorial page. I hope this is sufficient. Also added the note to the same section. Here's the link to the preview of the section: https://nvidia.github.io/NeMo-Guardrails/review/pr-1295/user-guides/configuration-guide/guardrails-configuration.html#parallel-execution-of-input-and-output-rails |
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
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.
Added some comments and suggestions. Main changes are to explicitly call out we validate the content-safety, topic control, and jailbreak to make sure it works. While other configurations may work, we don't validate them.
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Tim Gasser <[email protected]> Signed-off-by: Miyoung Choi <[email protected]>
@Pouyanpi @tgasser-nv incorporated feedback from both of you. Please take a look and approve so that I can merge this now. Thanks! |
docs/user-guides/configuration-guide/guardrails-configuration.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Pouyan <[email protected]> Signed-off-by: Miyoung Choi <[email protected]>
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.
LGTM, thanks for making the changes
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 @miyoungc 👍🏻
Description
https://docs.google.com/document/d/1xV0uSvv2G5iYLK4nz0e0T7VL--bTf0hKlQnhePRJqB0/edit?tab=t.0
Related Issue(s)
none
Checklist