Skip to content

Commit 3db4e2b

Browse files
GiggleLiuclaude
andcommitted
fix: size_field_names returns source's own fields, not target's
size_field_names() was returning output_size field names from the first matching reduction entry, which are the TARGET problem's fields when the queried problem is a source. Now correctly extracts input variable names when the problem is a source, and collects from all entries for completeness. Also tightens CLI inspect test to assert actual field names, and adds a cross-check test verifying all overhead variables are valid source size fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a64f060 commit 3db4e2b

3 files changed

Lines changed: 99 additions & 6 deletions

File tree

problemreductions-cli/tests/cli_tests.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2093,7 +2093,22 @@ fn test_inspect_json_output() {
20932093
let json: serde_json::Value = serde_json::from_str(&content).unwrap();
20942094
assert_eq!(json["kind"], "problem");
20952095
assert_eq!(json["type"], "MaximumIndependentSet");
2096-
assert!(json["size_fields"].is_array());
2096+
let size_fields: Vec<&str> = json["size_fields"]
2097+
.as_array()
2098+
.expect("size_fields should be an array")
2099+
.iter()
2100+
.map(|v| v.as_str().unwrap())
2101+
.collect();
2102+
assert!(
2103+
size_fields.contains(&"num_vertices"),
2104+
"MIS size_fields should contain num_vertices, got: {:?}",
2105+
size_fields
2106+
);
2107+
assert!(
2108+
size_fields.contains(&"num_edges"),
2109+
"MIS size_fields should contain num_edges, got: {:?}",
2110+
size_fields
2111+
);
20972112
assert!(json["solvers"].is_array());
20982113
assert!(json["reduces_to"].is_array());
20992114

src/rules/graph.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -624,19 +624,25 @@ impl ReductionGraph {
624624
/// Get the problem size field names for a problem type.
625625
///
626626
/// Derives size fields from the overhead expressions of reduction entries
627-
/// where this problem appears as source or target.
627+
/// where this problem appears as source or target. When the problem is a
628+
/// source, its size fields are the input variables referenced in the overhead
629+
/// expressions. When it's a target, its size fields are the output field names.
628630
pub fn size_field_names(&self, name: &str) -> Vec<&'static str> {
631+
let mut fields = std::collections::HashSet::new();
629632
for entry in inventory::iter::<ReductionEntry> {
630633
if entry.source_name == name {
631-
let overhead = entry.overhead();
632-
return overhead.output_size.iter().map(|(name, _)| *name).collect();
634+
// Source's size fields are the input variables of the overhead.
635+
fields.extend(entry.overhead().input_variable_names());
633636
}
634637
if entry.target_name == name {
638+
// Target's size fields are the output field names.
635639
let overhead = entry.overhead();
636-
return overhead.output_size.iter().map(|(name, _)| *name).collect();
640+
fields.extend(overhead.output_size.iter().map(|(name, _)| *name));
637641
}
638642
}
639-
vec![]
643+
let mut result: Vec<&'static str> = fields.into_iter().collect();
644+
result.sort_unstable();
645+
result
640646
}
641647

642648
/// Get all incoming reductions to a problem (across all its variants).

src/unit_tests/rules/graph.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::models::optimization::QUBO;
44
use crate::models::set::MaximumSetPacking;
55
use crate::rules::cost::MinimizeSteps;
66
use crate::rules::graph::{classify_problem_category, ReductionStep};
7+
use crate::rules::registry::ReductionEntry;
78
use crate::topology::SimpleGraph;
89
use crate::traits::Problem;
910
use crate::types::ProblemSize;
@@ -979,3 +980,74 @@ fn test_reduction_chain_with_variant_casts() {
979980
// Verify the extracted solution satisfies the original 3-SAT formula
980981
assert!(ksat.evaluate(&original_solution));
981982
}
983+
984+
#[test]
985+
fn test_size_field_names_returns_own_fields() {
986+
let graph = ReductionGraph::new();
987+
988+
// MIS should report its own fields (num_vertices, num_edges),
989+
// not the target's fields from any reduction.
990+
let mis_fields = graph.size_field_names("MaximumIndependentSet");
991+
assert!(
992+
mis_fields.contains(&"num_vertices"),
993+
"MIS should have num_vertices, got: {:?}",
994+
mis_fields
995+
);
996+
assert!(
997+
mis_fields.contains(&"num_edges"),
998+
"MIS should have num_edges, got: {:?}",
999+
mis_fields
1000+
);
1001+
// Should NOT contain target fields like num_vars or num_constraints
1002+
assert!(
1003+
!mis_fields.contains(&"num_constraints"),
1004+
"MIS should not report ILP's num_constraints, got: {:?}",
1005+
mis_fields
1006+
);
1007+
1008+
// QUBO should report num_vars
1009+
let qubo_fields = graph.size_field_names("QUBO");
1010+
assert!(
1011+
qubo_fields.contains(&"num_vars"),
1012+
"QUBO should have num_vars, got: {:?}",
1013+
qubo_fields
1014+
);
1015+
1016+
// Unknown problem returns empty
1017+
let unknown_fields = graph.size_field_names("NonExistentProblem");
1018+
assert!(unknown_fields.is_empty());
1019+
}
1020+
1021+
#[test]
1022+
fn test_overhead_variables_are_consistent() {
1023+
// For each reduction, the input variables of the overhead should be
1024+
// a subset of the source problem's size fields (as derived from all
1025+
// reductions where it appears).
1026+
let graph = ReductionGraph::new();
1027+
1028+
for entry in inventory::iter::<ReductionEntry> {
1029+
let overhead = entry.overhead();
1030+
let input_vars = overhead.input_variable_names();
1031+
if input_vars.is_empty() {
1032+
continue;
1033+
}
1034+
1035+
let source_fields: std::collections::HashSet<&str> = graph
1036+
.size_field_names(entry.source_name)
1037+
.into_iter()
1038+
.collect();
1039+
1040+
for var in &input_vars {
1041+
assert!(
1042+
source_fields.contains(var),
1043+
"Reduction {} -> {}: overhead references variable '{}' \
1044+
which is not a known size field of {}. Known fields: {:?}",
1045+
entry.source_name,
1046+
entry.target_name,
1047+
var,
1048+
entry.source_name,
1049+
source_fields
1050+
);
1051+
}
1052+
}
1053+
}

0 commit comments

Comments
 (0)