`json`: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

ochafik · 2024-06-06T09:30:10Z

Building upon #6640, I went on a wild goose chase wondering whether one could support numeric bounds with grammars, and I'm relatively happy with the provisional outcome.

{"type": "integer", "minimum": 0}
root ::= ([0] | [1-9] [0-9]{0,15}) space

{"type": "integer", "minimum": 3}
root ::= ([1-2] [0-9]{1,15} | [3-9] [0-9]{0,15}) space

{"type": "integer", "minimum": -123, "maximum": 42}
root ::= ("-" ([0-9] | ([1-8] [0-9] | [9] [0-9]) | "1" ([0-1] [0-9] | [2] [0-3])) | [0-9] | ([1-3] [0-9] | [4] [0-2])) space

This is still a WIP, here are some TODOs:

Increase test coverage
Fix some of the heading zeros (& bad numbers) allowed
Port to JS
Port to Python
Benchmark the resulting grammars (how much do we pay for this?)

Possible follow ups:

Add decimal numbers

/cc @HanClinto this since you mentioned it in #7789 :-D

p-e-w · 2024-06-09T04:22:35Z

{"type": "integer", "minimum": 3}
root ::= ([0-2] [0-9]{1,15} | [3-9] [0-9]{0,15}) space

This rule matches strings like 01, which are neither valid JSON integers nor greater than or equal to 3.

ochafik · 2024-06-09T08:44:19Z

{"type": "integer", "minimum": 3}
root ::= ([0-2] [0-9]{1,15} | [3-9] [0-9]{0,15}) space
This rule matches strings like 01, which are neither valid JSON integers nor greater than or equal to 3.

@p-e-w Oops good catch, thanks! That was one of the "heading zeros allowed" I had on my radar, hadn't realized it was also letting through bad numbers. Fixed.

@ochafik

* Adding simple bare-bones test for end-to-end integration test for json validation against auto-generated JSON-schema grammars. * Adding additional examples as documented in #7789 . Also adding the ability to automatically output improperly failing grammars to debug output files so they can more easily be examined in the gbnf-validator program. * Uncommenting formerly commented tests so that they fail for others who are attempting to reproduce the bugs. * Merging improved schema test methods added by @ochafik in #7797 * Adding #define to temporarily remove failing tests so that this PR can pass CI, but still be useful for other PRs that want to leverage the framework. * Fixing nits from ochafik. Removing escape slashes, adding additional failing cases, fixing some other strings. * Fixing grammar indentation to be consistent throughout file.

HanClinto · 2024-06-24T17:36:51Z

tests/test-grammar-integration.cpp

+    );
+
+    test_schema(
+        "min 0",


This schema looks identical with the "simple min 0" test from line 154. Can they be merged together to shorten this file?

HanClinto · 2024-06-24T17:37:32Z

tests/test-grammar-integration.cpp

@@ -800,6 +948,165 @@ static void test_json_schema() {
        }
    );

+    test_schema(
+        "min -123 max 42",


This schema looks identical to the one from the "simple min -123 max 42" test on line 249. Can they be merged together?

Oops, reshuffled / merged them all, thanks!

HanClinto · 2024-06-24T21:35:53Z

common/json-schema-to-grammar.cpp

@@ -16,7 +16,7 @@ static std::string join(Iterator begin, Iterator end, const std::string & separa

 static std::string repeat(const std::string & str, size_t n);

-static std::string build_repetition(const std::string & item_rule, int min_items, int max_items, const std::string & separator_rule = "") {
+static std::string build_repetition(const std::string & item_rule, const int min_items, int max_items, const std::string & separator_rule = "") {


If you're changing min_items to const, is it worth adding const to max_items as well?

Dropped unintended change, thanks!

HanClinto · 2024-06-24T21:39:32Z

tests/test-grammar-integration.cpp

@@ -12,6 +12,9 @@
 #include <cassert>
 #include <string>
 #include <vector>
+#include <json-schema-to-grammar.h>
+
+using json = nlohmann::ordered_json;


This might be a fault of the way I merged the code, but we don't need these 3 lines because json-schema-to-grammar.h is already included, and this using line is already here.

Oops sorry bad merge, fixed

HanClinto

Looks great!

…aviour of libstdc++ vs. clang's libc++, can't read final NULL char w/ former)

HanClinto

Nice defensive programming!

HanClinto · 2024-06-25T19:09:44Z

Very excited to see these changes getting merged in!! :) 🚀 🚀 🚀

@ochafik

* Adding simple bare-bones test for end-to-end integration test for json validation against auto-generated JSON-schema grammars. * Adding additional examples as documented in ggerganov#7789 . Also adding the ability to automatically output improperly failing grammars to debug output files so they can more easily be examined in the gbnf-validator program. * Uncommenting formerly commented tests so that they fail for others who are attempting to reproduce the bugs. * Merging improved schema test methods added by @ochafik in ggerganov#7797 * Adding #define to temporarily remove failing tests so that this PR can pass CI, but still be useful for other PRs that want to leverage the framework. * Fixing nits from ochafik. Removing escape slashes, adding additional failing cases, fixing some other strings. * Fixing grammar indentation to be consistent throughout file.

…Maximum (ggerganov#7797) * json: support minimum for positive integer values * json: fix min 0 * json: min + max integer constraints * json: handle negative min / max integer bounds * json: fix missing paren min/max bug * json: proper paren fix * json: integration test for schemas * json: fix bounds tests * Update json-schema-to-grammar.cpp * json: fix negative max * json: fix negative min (w/ more than 1 digit) * Update test-grammar-integration.cpp * json: nit: move string rules together * json: port min/max integer support to Python & JS * nit: move + rename _build_min_max_int * fix min in [1, 9] * Update test-grammar-integration.cpp * add C++11-compatible replacement for std::string_view * add min/max constrained int field to pydantic json schema example * fix merge * json: add integration tests for min/max bounds * reshuffle/merge min/max integ test cases * nits / cleanups * defensive code against string out of bounds (apparently different behaviour of libstdc++ vs. clang's libc++, can't read final NULL char w/ former)

ochafik added 9 commits June 6, 2024 10:14

json: support minimum for positive integer values

057bbdc

json: fix min 0

d69ccb0

json: min + max integer constraints

c37c484

json: handle negative min / max integer bounds

af63f4f

json: fix missing paren min/max bug

a381deb

json: proper paren fix

f8db478

json: integration test for schemas

5a86c6f

json: fix bounds tests

431edb8

Update json-schema-to-grammar.cpp

b6b6a6c

ochafik mentioned this pull request Jun 6, 2024

JSON Schema to GBNF integration tests #7790

Merged

github-actions bot added the testing Everything test related label Jun 6, 2024

ochafik added enhancement New feature or request Review Complexity : High Generally require indepth knowledge of LLMs or GPUs and removed testing Everything test related labels Jun 6, 2024

Merge remote-tracking branch 'origin/master' into json-bounds2

a786c03

github-actions bot added the testing Everything test related label Jun 8, 2024

ochafik added 5 commits June 8, 2024 20:33

json: fix negative max

931b543

json: fix negative min (w/ more than 1 digit)

4c1c293

Update test-grammar-integration.cpp

ac2a8f8

json: nit: move string rules together

3549702

json: port min/max integer support to Python & JS

e933680

github-actions bot added examples python python script changes server labels Jun 8, 2024

nit: move + rename _build_min_max_int

a0f1904

fix min in [1, 9]

dcc27d1

ochafik added 2 commits June 9, 2024 13:22

Update test-grammar-integration.cpp

d1f6791

add C++11-compatible replacement for std::string_view

cad377d

mofosyne added the Review Complexity : Medium Generally require more time to grok but manageable by beginner to medium expertise level label Jun 12, 2024

HanClinto added a commit to HanClinto/llama.cpp that referenced this pull request Jun 12, 2024

Merging improved schema test methods added by @ochafik in ggerganov#7797

cfccb6c

Merge remote-tracking branch 'origin/master' into json-bounds2

f03e9b9

HanClinto added a commit to HanClinto/llama.cpp that referenced this pull request Jun 12, 2024

Merging improved schema test methods added by @ochafik in ggerganov#7797

6ccb1fb

HanClinto added a commit to HanClinto/llama.cpp that referenced this pull request Jun 22, 2024

Merging improved schema test methods added by @ochafik in ggerganov#7797

d4a63b0

ochafik added 4 commits June 22, 2024 19:47

Merge remote-tracking branch 'origin/master' into json-bounds2

6fa7364

fix merge

948e55e

json: add integration tests for min/max bounds

670d5a6

Merge remote-tracking branch 'origin/master' into json-bounds2

9fb8a75

ochafik changed the title ~~[WIP] json: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum~~ json: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum Jun 24, 2024

ochafik marked this pull request as ready for review June 24, 2024 00:43

HanClinto reviewed Jun 24, 2024

View reviewed changes

ochafik added 2 commits June 24, 2024 21:21

Merge remote-tracking branch 'origin/master' into json-bounds2

d7d957d

reshuffle/merge min/max integ test cases

3a80d1e

HanClinto reviewed Jun 24, 2024

View reviewed changes

ochafik mentioned this pull request Jun 24, 2024

json: fix additionalProperties, allow space after enum/const #7840

Merged

2 tasks

nits / cleanups

09a9b75

HanClinto approved these changes Jun 24, 2024

View reviewed changes

ochafik added 2 commits June 25, 2024 02:11

Merge remote-tracking branch 'origin/master' into json-bounds2

48f417d

defensive code against string out of bounds (apparently different beh…

36bf003

…aviour of libstdc++ vs. clang's libc++, can't read final NULL char w/ former)

HanClinto reviewed Jun 25, 2024

View reviewed changes

ochafik merged commit 84631fe into ggerganov:master Jun 25, 2024
62 of 63 checks passed

ochafik deleted the json-bounds2 branch June 28, 2024 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`json`: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

`json`: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

ochafik commented Jun 6, 2024 •

edited

Loading

p-e-w commented Jun 9, 2024

ochafik commented Jun 9, 2024

HanClinto Jun 24, 2024

ochafik Jun 24, 2024

HanClinto Jun 24, 2024

ochafik Jun 24, 2024

HanClinto Jun 24, 2024

ochafik Jun 24, 2024

HanClinto Jun 24, 2024

ochafik Jun 24, 2024

HanClinto left a comment

HanClinto left a comment

HanClinto commented Jun 25, 2024 •

edited

Loading

json: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

json: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

Conversation

ochafik commented Jun 6, 2024 • edited Loading

p-e-w commented Jun 9, 2024

ochafik commented Jun 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HanClinto left a comment

Choose a reason for hiding this comment

HanClinto left a comment

Choose a reason for hiding this comment

HanClinto commented Jun 25, 2024 • edited Loading

`json`: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

`json`: support integer minimum, maximum, exclusiveMinimum, exclusiveMaximum #7797

ochafik commented Jun 6, 2024 •

edited

Loading

HanClinto commented Jun 25, 2024 •

edited

Loading