Add parity tests against Julia ProblemReductions.jl#65
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Generates JSON fixtures from ProblemReductions.jl for parity testing: - 10 model fixtures (IndependentSet, SpinGlass, MaxCut, QUBO, SAT, KSat, VertexCovering, SetPacking, Matching, Factoring) - 17 reduction fixtures covering all Julia test/rules/rules.jl pairs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
22 active tests covering: - 10 model evaluations (IS, SpinGlass, MaxCut, QUBO, SAT, KSat, VertexCover, SetPacking, Matching, Factoring) - 12 reduction closed-loop tests (IS↔SetPacking, IS→VC, VC→SetCovering, SpinGlass↔MaxCut, SpinGlass↔QUBO, SAT↔KSat, CircuitSAT→SpinGlass, Factoring→CircuitSAT) - 4 ignored stubs for unimplemented reductions (SAT→Coloring, SAT→IS, SAT→DominatingSet, Matching→SetPacking) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
==========================================
- Coverage 97.08% 97.06% -0.02%
==========================================
Files 186 187 +1
Lines 25643 25150 -493
==========================================
- Hits 24895 24412 -483
+ Misses 748 738 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "label": "circuit", | ||
| "extracted_single": [ | ||
| [ | ||
| 1, |
There was a problem hiding this comment.
no line break in the dumped json file.
There was a problem hiding this comment.
Fixed in b28ea0e — JSON fixtures now use compact format and files reorganized to tests/data/jl/.
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive parity testing between the Rust problemreductions crate and Julia's ProblemReductions.jl library to ensure behavioral consistency. The implementation includes a Julia test data generation script that produces JSON fixtures and corresponding Rust integration tests that verify matching results.
Changes:
- Added Julia local environment with ProblemReductions.jl dependencies
- Implemented Julia script to generate 26 JSON test fixtures covering 10 problem types and 16 reduction paths
- Added comprehensive Rust parity tests for model evaluations, solver results, and reduction round-trips
- Integrated new test suite into existing test infrastructure with
make jl-testdatatarget
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/jl/Project.toml | Julia project dependencies for ProblemReductions.jl, Graphs, and JSON |
| scripts/jl/Manifest.toml | Locked Julia dependency versions |
| scripts/jl/generate_testdata.jl | Julia script to generate test fixtures with proper 1→0-based index conversion |
| tests/suites/jl_parity.rs | Rust parity tests for 10 problem types and 12 active reduction paths |
| tests/main.rs | Integration of jl_parity test module |
| tests/data/jl_*.json | 26 JSON fixture files (10 model fixtures, 16 reduction fixtures) |
| Makefile | Added jl-testdata target for fixture regeneration |
| docs/plans/*.md | Design and implementation plan documents |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/suites/jl_parity.rs
Outdated
| fn parse_edges(instance: &serde_json::Value) -> Vec<(usize, usize)> { | ||
| instance["edges"] | ||
| .as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|e| { | ||
| let arr = e.as_array().unwrap(); | ||
| ( | ||
| arr[0].as_u64().unwrap() as usize, | ||
| arr[1].as_u64().unwrap() as usize, | ||
| ) | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| fn parse_weighted_edges(instance: &serde_json::Value) -> Vec<(usize, usize, i32)> { | ||
| let edges = parse_edges(instance); | ||
| let weights: Vec<i32> = instance["weights"] | ||
| .as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|w| w.as_i64().unwrap() as i32) | ||
| .collect(); | ||
| edges | ||
| .into_iter() | ||
| .zip(weights) | ||
| .map(|((u, v), w)| (u, v, w)) | ||
| .collect() | ||
| } | ||
|
|
||
| fn parse_config(val: &serde_json::Value) -> Vec<usize> { | ||
| val.as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|v| v.as_u64().unwrap() as usize) | ||
| .collect() | ||
| } | ||
|
|
||
| fn parse_configs_set(val: &serde_json::Value) -> HashSet<Vec<usize>> { | ||
| val.as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(parse_config) | ||
| .collect() | ||
| } | ||
|
|
||
| fn parse_i32_vec(val: &serde_json::Value) -> Vec<i32> { | ||
| val.as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|v| v.as_i64().unwrap() as i32) | ||
| .collect() | ||
| } | ||
|
|
||
| fn parse_sets(val: &serde_json::Value) -> Vec<Vec<usize>> { | ||
| val.as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|s| { | ||
| s.as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|v| v.as_u64().unwrap() as usize) | ||
| .collect() | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| fn parse_sat_clauses(instance: &serde_json::Value) -> (usize, Vec<CNFClause>) { | ||
| let num_vars = instance["num_variables"].as_u64().unwrap() as usize; | ||
| let clauses: Vec<CNFClause> = instance["clauses"] | ||
| .as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|clause| { | ||
| let literals: Vec<i32> = clause["literals"] | ||
| .as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|lit| { | ||
| let var = lit["variable"].as_u64().unwrap() as i32 + 1; // Convert to 1-indexed | ||
| let negated = lit["negated"].as_bool().unwrap(); | ||
| if negated { -var } else { var } | ||
| }) | ||
| .collect(); | ||
| CNFClause::new(literals) | ||
| }) | ||
| .collect(); | ||
| (num_vars, clauses) | ||
| } |
There was a problem hiding this comment.
The test functions extensively use .unwrap() for JSON deserialization and parsing. While this is acceptable for tests (where panics are tolerable), consider adding more descriptive error messages to help debug when fixtures don't match expected format. For example, instance["instance"]["num_vertices"].as_u64().expect("num_vertices should be a u64") would be more helpful than just .unwrap() when the JSON structure is incorrect.
There was a problem hiding this comment.
Fixed in b28ea0e — replaced .unwrap() with descriptive .expect() messages in all JSON parsing helpers (parse_edges, parse_config, parse_i32_vec, etc.).
| true, | ||
| false | ||
| ], | ||
| [ | ||
| true, | ||
| true | ||
| ] | ||
| ], | ||
| "extracted_multiple": [ | ||
| [ | ||
| true, | ||
| false | ||
| ], | ||
| [ | ||
| true, | ||
| true | ||
| ], | ||
| [ | ||
| false, | ||
| true |
There was a problem hiding this comment.
The extracted_single and extracted_multiple fields in this fixture contain boolean values (true/false) instead of integers (1/0). This is inconsistent with other fixtures and will cause parsing errors in the Rust tests, which expect integer arrays via parse_config. This fixture appears to not be used by any Rust test (SAT → IndependentSet is marked as #[ignore]), so it doesn't cause immediate failures, but should be fixed for consistency.
| true, | |
| false | |
| ], | |
| [ | |
| true, | |
| true | |
| ] | |
| ], | |
| "extracted_multiple": [ | |
| [ | |
| true, | |
| false | |
| ], | |
| [ | |
| true, | |
| true | |
| ], | |
| [ | |
| false, | |
| true | |
| 1, | |
| 0 | |
| ], | |
| [ | |
| 1, | |
| 1 | |
| ] | |
| ], | |
| "extracted_multiple": [ | |
| [ | |
| 1, | |
| 0 | |
| ], | |
| [ | |
| 1, | |
| 1 | |
| ], | |
| [ | |
| 0, | |
| 1 |
There was a problem hiding this comment.
Fixed in b28ea0e — added bool_to_int conversion in export_reduction() so extract_solution results serialize as 0/1 integers instead of true/false. SAT→IndependentSet test is now active (no longer #[ignore]).
Systematic Comparison: Julia ProblemReductions.jl vs Rust Parity TestsModel Evaluation Coverage (16 Julia problems)
14/16 problems covered. BicliqueCover and BMF have no Julia doc examples to extract. Reduction Coverage (Julia has ~14 reduction rules)
10/14 reductions fully tested. 4 have fixtures generated but are Semantic Mapping Notes
|
- Reorganize fixtures: tests/data/jl_*.json → tests/data/jl/*.json with compact JSON - Add doc example instances for 5 new problem types (DominatingSet, MaximalIS, PaintShop, KColoring, SetCovering) and doc instances for existing problems - Add 22 individual rule test instances from Julia test/rules/*.jl covering spinglass↔maxcut, qubo→spinglass, vc→setcovering, is→setpacking, matching→setpacking, sat→ksat/coloring/independentset/dominatingset - Implement 11 new reduction parity tests (replaced 4 #[ignore] stubs) - Use ILP solver for SAT→Coloring test (brute force was 185s, now 0.02s) - Handle unsatisfiable SAT instances in evaluation test 39 passing, 1 ignored (SAT→CircuitSAT not in Rust) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix boolean values in extracted solutions (true/false → 0/1 integers) - Replace .unwrap() with descriptive .expect() messages in JSON parsers - Compact JSON already addressed in previous commit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All 40 Julia ProblemReductions.jl parity tests (model evaluations and reduction round-trips) are now in src/unit_tests/jl_parity.rs, removing duplication with the existing unit test suite. The integration test file tests/suites/jl_parity.rs is reduced to a stub comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit f3fc90b.
Distribute Julia parity tests from tests/suites/jl_parity.rs into
the corresponding unit test files (15 model + 12 rule files). Tests
use shared JSON parsing helpers via include!("../jl_helpers.rs").
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove ~95 hand-written evaluate/brute_force tests from model and rule unit test files that are fully covered by the JL fixture-based parity tests. Keep construction, utility, trait compliance, edge case, and relationship tests that verify distinct concerns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 89 out of 90 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/unit_tests/jl_helpers.rs
Outdated
| let num_vars = instance["num_variables"].as_u64().unwrap() as usize; | ||
| let clauses = instance["clauses"] | ||
| .as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|clause| { | ||
| let literals: Vec<i32> = clause["literals"] | ||
| .as_array() | ||
| .unwrap() | ||
| .iter() | ||
| .map(|lit| { | ||
| let var = lit["variable"].as_u64().unwrap() as i32 + 1; | ||
| let negated = lit["negated"].as_bool().unwrap(); |
There was a problem hiding this comment.
The jl_parse_sat_clauses function uses .unwrap() in several places (lines 85-97) instead of .expect() with descriptive messages, which is inconsistent with the rest of the helper functions that use .expect(). This makes debugging more difficult when JSON fixtures don't match the expected format. Consider replacing .unwrap() with .expect("...") to provide helpful error messages when the JSON structure is incorrect.
| let num_vars = instance["num_variables"].as_u64().unwrap() as usize; | |
| let clauses = instance["clauses"] | |
| .as_array() | |
| .unwrap() | |
| .iter() | |
| .map(|clause| { | |
| let literals: Vec<i32> = clause["literals"] | |
| .as_array() | |
| .unwrap() | |
| .iter() | |
| .map(|lit| { | |
| let var = lit["variable"].as_u64().unwrap() as i32 + 1; | |
| let negated = lit["negated"].as_bool().unwrap(); | |
| let num_vars = instance["num_variables"] | |
| .as_u64() | |
| .expect("num_variables should be a u64") as usize; | |
| let clauses = instance["clauses"] | |
| .as_array() | |
| .expect("clauses should be an array") | |
| .iter() | |
| .map(|clause| { | |
| let literals: Vec<i32> = clause["literals"] | |
| .as_array() | |
| .expect("clause.literals should be an array") | |
| .iter() | |
| .map(|lit| { | |
| let var = lit["variable"] | |
| .as_u64() | |
| .expect("literal.variable should be a u64") as i32 | |
| + 1; | |
| let negated = lit["negated"] | |
| .as_bool() | |
| .expect("literal.negated should be a bool"); |
src/unit_tests/jl_helpers.rs
Outdated
| .unwrap() | ||
| .iter() | ||
| .find(|inst| inst["label"].as_str().unwrap() == label) |
There was a problem hiding this comment.
The jl_find_instance_by_label function also uses .unwrap() on lines 124-127 instead of .expect(), which is inconsistent with the error handling pattern established in other helper functions. Consider using .expect() with a descriptive message for the array access to maintain consistency and improve debugging.
| .unwrap() | |
| .iter() | |
| .find(|inst| inst["label"].as_str().unwrap() == label) | |
| .expect("instances should be an array") | |
| .iter() | |
| .find(|inst| inst["label"].as_str().expect("instance label should be a string") == label) |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Resolves #64
scripts/jl/generate_testdata.jl) that exercises ProblemReductions.jl to produce JSON fixtures for problem construction, evaluation, and reductionstests/suites/jl_parity.rs) that deserialize the Julia fixtures and verify that the Rust implementations produce identical problem dimensions, evaluations, optimal solutions, and reduction round-trip resultsmake jl-testdatatarget to regenerate fixtures from Julia when neededscripts/jl/Project.toml,Manifest.toml) pinning ProblemReductions.jl dependenciesTest plan
make testpasses (includes the newjl_paritytest suite)make clippypassesmake jl-testdataregenerates fixtures (requires Julia + ProblemReductions.jl)🤖 Generated with Claude Code