-
Notifications
You must be signed in to change notification settings - Fork 4
Add Wishart synthesis metadata schema #111
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,16 @@ | |
| }, | ||
| "dwig_generator": { | ||
| "type": "string" | ||
| }, | ||
| "synthesis": { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Backwards-compatibility note: previously
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 548379c. I added an Unreleased changelog note calling out the stricter BQPJSON |
||
| "oneOf": [ | ||
| { | ||
| "$ref": "#/definitions/wishart_synthesis_metadata" | ||
| }, | ||
| { | ||
| "$ref": "#/definitions/generic_synthesis_metadata" | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| }, | ||
|
|
@@ -159,5 +169,59 @@ | |
| } | ||
| } | ||
| } | ||
| }, | ||
| "definitions": { | ||
| "generic_synthesis_metadata": { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This generic branch is the only thing keeping non-Wishart synthesis metadata valid after the tightening (e.g. Sherrington-Kirkpatrick emits
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 548379c. I added a Sherrington-Kirkpatrick BQPJSON write/read regression test in |
||
| "type": "object", | ||
| "required": [ | ||
| "model", | ||
| "parameters" | ||
| ], | ||
| "properties": { | ||
| "model": { | ||
| "type": "string", | ||
| "not": { | ||
| "enum": [ | ||
| "Wishart" | ||
| ] | ||
| } | ||
| }, | ||
| "parameters": { | ||
| "type": "object" | ||
| } | ||
| } | ||
| }, | ||
| "wishart_synthesis_metadata": { | ||
| "type": "object", | ||
| "required": [ | ||
| "model", | ||
| "parameters" | ||
| ], | ||
| "properties": { | ||
| "model": { | ||
| "type": "string", | ||
| "enum": [ | ||
| "Wishart" | ||
| ] | ||
| }, | ||
| "parameters": { | ||
| "type": "object", | ||
| "required": [ | ||
| "n", | ||
| "m" | ||
| ], | ||
| "properties": { | ||
| "n": { | ||
| "type": "integer", | ||
| "minimum": 0 | ||
| }, | ||
| "m": { | ||
| "type": "integer", | ||
| "minimum": 0 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,59 @@ function test_bqpjson_format() | |
| QUBOTools.value(dst_model, [1, 1, 0]) | ||
| end | ||
| end | ||
|
|
||
| @testset "Wishart synthesis metadata" begin | ||
| n = 6 | ||
| m = 3 | ||
|
|
||
| src_model = @test_logs (:warn, r"Deprecation Warning:.*QUBOLib") QUBOTools.generate( | ||
| Random.MersenneTwister(110), | ||
| QUBOTools.Wishart(n, m), | ||
| ) | ||
| synthesis = QUBOTools.metadata(src_model)["synthesis"] | ||
|
|
||
| @test synthesis["model"] == "Wishart" | ||
| @test synthesis["parameters"] == Dict{String,Any}( | ||
| "n" => n, | ||
| "m" => m, | ||
| ) | ||
|
|
||
| _with_temp_path("wishart.bool.json") do temp_path | ||
| QUBOTools.write_model(temp_path, src_model) | ||
|
|
||
| dst_model = QUBOTools.read_model(temp_path) | ||
|
|
||
| @test QUBOTools.metadata(dst_model)["synthesis"] == synthesis | ||
| end | ||
| end | ||
|
|
||
| @testset "Wishart synthesis schema validation" begin | ||
| _with_temp_path("invalid-wishart.bool.json") do temp_path | ||
| write(temp_path, """ | ||
| { | ||
| "version": "1.0.0", | ||
| "id": 0, | ||
| "variable_ids": [1], | ||
| "variable_domain": "boolean", | ||
| "scale": 1.0, | ||
| "offset": 0.0, | ||
| "linear_terms": [], | ||
| "quadratic_terms": [], | ||
| "metadata": { | ||
| "synthesis": { | ||
| "model": "Wishart", | ||
| "parameters": { | ||
| "n": "6", | ||
| "m": 3 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| """) | ||
|
|
||
| @test_throws QUBOTools.FormatError QUBOTools.read_model(temp_path) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This asserts only the error type, not that the failure comes from the synthesis block. The fixture is otherwise valid so it currently fails for the right reason, but an unrelated future change to this JSON could make it pass for the wrong reason. Consider asserting the error message references
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 548379c. The negative Wishart test now catches the |
||
| end | ||
| end | ||
| end | ||
|
|
||
| return nothing | ||
|
|
||
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.
Only the Wishart shape is documented. The generic contract now enforced for every other generator (
model= any string other thanWishart, plus aparametersobject) gates reads but is undocumented. Consider adding a short note. Nonblocking.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.
Addressed in 548379c. The BQPJSON docs now describe the generic non-Wishart
metadata.synthesiscontract and keep the Wishart-specific shape documented separately.