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.

Sign up for GitHub

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

Jump to bottom

[feat] Add json constraint decoding to core-schema-impl/0.5.0 branch. #835

Merged
merged 52 commits into from
Jun 20, 2024
Merged

[feat] Add json constraint decoding to core-schema-impl/0.5.0 branch. #835

merged 52 commits into from
Jun 20, 2024

Conversation

JosephCatrambone
Copy link
Contributor

@JosephCatrambone JosephCatrambone commented Jun 14, 2024 •
edited
Loading

Adds the formatters module and the JSONFormatter module, which in turn uses JSONFormer to better constrain the output of huggingface based models.

Performance delta on a 2023 MacBook Pro with Apple M3 Max:

  • With formatter: 1.48 s ± 23.4 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
  • Without formatter: 738 ms ± 130 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

These deltas may be a result of...

  • Increased prompt size when using the JSONFormatter.
  • 'Without Formatter' breaking early because of an invalid JSON return. (I.e., rather than iterate multiple times to complete generation, it breaks as soon as invalid JSON is encountered. This is consistent with observations of the returned data.)

Further investigation is planned. Additionally, tests currently are using gpt-2 as a sample model, but we shouldn't be doing any HF imports in the tests. When core-schema-impl is merged we will have access to the new testing harnesses, including the mock models.

@JosephCatrambone
Mock out constraint generator and add to HF model callbacks.
b6d7ec7
@JosephCatrambone
Starting on basic integration test.
c31be18
@JosephCatrambone
Testing fixes. Use BatchEncoding for the mocker to smooth things out.
fb8973c
@JosephCatrambone
Update constraint generators, adding many more kinds of constraints.
1cad845
@JosephCatrambone
Additional constraints.
1d530dc
@JosephCatrambone
Feeble third attempt at integrating JSON constraints.
7957180
@JosephCatrambone
Checkpointing before task switch.
894ed07
@JosephCatrambone
Partial success. Needs a great deal of work.
d86d156
@JosephCatrambone
Start on pushing the output formatter up through the stack. Make thin…
7fcef2d 
…gs work for pipelines.
@JosephCatrambone
Make test slightly more complicated.
87d86a8
@JosephCatrambone
Lift formatters into a 'formatters' directory. Feels slightly cleaner…
662f604 
…, but not perfect.
@JosephCatrambone
Add converter for JSONSchema to JSONFormer style.
a3149a0
@JosephCatrambone
Update pyproject with the new jsonformer dependency.
4abb721
@JosephCatrambone
Add unit tests for the schema converter. Move the formatter tests fro…
4b6532e 
…m the guard to a dedicated file for formatters.
@JosephCatrambone
Merge branch 'core-schema-impl' into jc/feature-add-json-constraint-c…
341c45b 
…ore-schema-impl
@JosephCatrambone
Remove unused file.
c752e2b
Base automatically changed from core-schema-impl to 0.5.0-dev June 17, 2024 13:05
@notion-workspace Notion Workspace
Copy link

Constraint Generation Standalone Class

@JosephCatrambone JosephCatrambone marked this pull request as ready for review June 18, 2024 16:52
@JosephCatrambone JosephCatrambone changed the title Feature: Add json constraint decoding to core-schema-impl/0.5.0 branch. [feat] Add json constraint decoding to core-schema-impl/0.5.0 branch. Jun 18, 2024
CalebCourier
CalebCourier reviewed Jun 18, 2024
View reviewed changes
guardrails/guard.py
@@ -496,6 +500,14 @@ def from_pydantic(
guard._exec_opts = exec_opts
guard._output_type = schema.output_type
guard._base_model = output_class
if isinstance(output_formatter, str):
if isinstance(output_class, list):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do support root level lists for other flows. Is this restriction because of a limitation with JSONFormer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. :(

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fair enough, let's make the error message more specific then to explicitly state that output_formatter="jsonformer" can't be used with root-level lists.

Copy link
Collaborator

Choose a reason for hiding this comment

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

btw root-level lists are apparently an anti-pattern from an OpenAI function calling perspective - they only support top-level object for structured data

Copy link
Contributor Author

@JosephCatrambone JosephCatrambone Jun 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It turns out they're valid JSON, though! I did not know that. I've edited the message to say the following:

Root-level arrays are not supported with the jsonformer argument, but can be used with other json generation methods. Omit the output_formatter argument to use the other methods.

Copy link
Collaborator

Choose a reason for hiding this comment

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

btw root-level lists are apparently an anti-pattern from an OpenAI function calling perspective - they only support top-level object for structured data

@zsimjee
Is this a new limitation with tools vs the old functions args? We've been using function calling with top level arrays with that syntax for a while now

@CalebCourier CalebCourier mentioned this pull request Jun 18, 2024
Merged
24 tasks
@JosephCatrambone
Merge branch 'jc/feature-add-json-constraint-core-schema-impl' of git…
b35cb39 
…hub.com:guardrails-ai/guardrails into jc/feature-add-json-constraint-core-schema-impl
@JosephCatrambone
Update error message and fix unrelated linting error.
020fb90
@JosephCatrambone
Merge branch 'jc/feature-add-json-constraint-core-schema-impl' of git…
85cee1f 
…hub.com:guardrails-ai/guardrails into jc/feature-add-json-constraint-core-schema-impl
@JosephCatrambone
The unused import is absolutely used. Add 'noqa: F401'.
9e36480
@JosephCatrambone
Merge branch 'jc/feature-add-json-constraint-generator' of github.com…
a77f4ad 
…:guardrails-ai/guardrails into jc/feature-add-json-constraint-core-schema-impl
@JosephCatrambone
Merge branch '0.5.0-dev' of github.com:guardrails-ai/guardrails into …
30b681e 
…jc/feature-add-json-constraint-core-schema-impl
@JosephCatrambone
Subtest has moved around. Fix.
6307ea9
@JosephCatrambone
Regenerate poetry lock.
6177127
@JosephCatrambone
Revert "Merge branch 'jc/feature-add-json-constraint-generator' of gi…
c62abaf 
…thub.com:guardrails-ai/guardrails into jc/feature-add-json-constraint-core-schema-impl"

This reverts commit a77f4ad, reversing
changes made to 9e36480.
@JosephCatrambone
import openai # noqa: F401 because openai is used but not detected as…
78f109b 
… used.
@JosephCatrambone JosephCatrambone merged commit 9c7b5d2 into 0.5.0-dev Jun 20, 2024
12 checks passed
@JosephCatrambone JosephCatrambone deleted the jc/feature-add-json-constraint-core-schema-impl branch June 20, 2024 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zsimjee zsimjee zsimjee left review comments

@CalebCourier CalebCourier CalebCourier approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JosephCatrambone @zsimjee @CalebCourier