Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions rust/kcl-lib/src/execution/exec_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1162,13 +1162,27 @@ impl Node<SketchBlock> {
// Solve constraints.
let config = kcl_ezpz::Config::default().with_max_iterations(50);
let solve_result = if exec_state.mod_local.freedom_analysis {
kcl_ezpz::solve_analysis(&constraints, initial_guesses.clone(), config)
.map(|outcome| (outcome.outcome, Some(FreedomAnalysis::from(outcome.analysis))))
kcl_ezpz::solve_analysis(&constraints, initial_guesses.clone(), config).map(|outcome| {
let freedom_analysis = FreedomAnalysis::from(outcome.analysis);
(outcome.outcome, Some(freedom_analysis))
})
} else {
kcl_ezpz::solve(&constraints, initial_guesses.clone(), config).map(|outcome| (outcome, None))
};
// Build a combined list of all constraints (regular + optional) for conflict detection
let num_required_constraints = sketch_block_state.solver_constraints.len();
let all_constraints: Vec<kcl_ezpz::Constraint> = sketch_block_state
.solver_constraints
.iter()
.cloned()
.chain(sketch_block_state.solver_optional_constraints.iter().cloned())
.collect();

let (solve_outcome, solve_analysis) = match solve_result {
Ok((solved, freedom)) => (Solved::from(solved), freedom),
Ok((solved, freedom)) => (
Solved::from_ezpz_outcome(solved, &all_constraints, num_required_constraints),
freedom,
),
Err(failure) => {
match &failure.error {
NonLinearSystemError::FaerMatrix { .. }
Expand All @@ -1190,6 +1204,7 @@ impl Node<SketchBlock> {
warnings: failure.warnings,
unsatisfied: Default::default(),
priority_solved: Default::default(),
variables_in_conflicts: Default::default(),
},
None,
)
Expand Down
183 changes: 183 additions & 0 deletions rust/kcl-lib/src/execution/freedom_analysis_tests.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added these tests failing before I started working on a fix, now they pass.

Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
#[cfg(test)]
#[cfg(feature = "artifact-graph")]
mod test {
use std::sync::Arc;

use crate::{
ExecutorContext, ExecutorSettings,
engine::conn_mock::EngineConnection,
execution::{ContextType, MockConfig},
front::{Freedom, ObjectKind},
frontend::api::ObjectId,
};

async fn run_with_freedom_analysis(kcl: &str) -> Vec<(ObjectId, Freedom)> {
let program = crate::Program::parse_no_errs(kcl).unwrap();

let exec_ctxt = ExecutorContext {
engine: Arc::new(Box::new(EngineConnection::new().unwrap())),
fs: Arc::new(crate::fs::FileManager::new()),
settings: ExecutorSettings::default(),
context_type: ContextType::Mock,
};

let mock_config = MockConfig {
freedom_analysis: true,
..Default::default()
};

let outcome = exec_ctxt.run_mock(&program, &mock_config).await.unwrap();

let mut point_freedoms = Vec::new();
for obj in &outcome.scene_objects {
if let ObjectKind::Segment {
segment: crate::front::Segment::Point(point),
} = &obj.kind
{
point_freedoms.push((obj.id, point.freedom));
}
}
// Sort by object ID for consistent ordering
point_freedoms.sort_by_key(|(id, _)| id.0);
point_freedoms
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add this? It's a no-op for mock contexts, but I want us to get in the habit of closing because it's a bug for an engine context.

exec_ctxt.close().await;


#[tokio::test(flavor = "multi_thread")]
async fn test_freedom_analysis_with_conflicts() {
let kcl = r#"
@settings(experimentalFeatures = allow)
sketch(on = YZ) {
line1 = sketch2::line(start = [var 2mm, var 8mm], end = [var 5mm, var 7mm])
line1.start.at[0] == 2
line1.start.at[1] == 8
line1.end.at[0] == 5
line1.end.at[1] == 7
line2 = sketch2::line(start = [var 2mm, var 1mm], end = [var -4.75mm, var -0.88mm])
line2.start.at[0] == 2
line2.start.at[1] == 1
line3 = sketch2::line(start = [var -2.591mm, var -7.081mm], end = [var 1.331mm, var -3.979mm])
sketch2::distance([line3.start, line3.end]) == 4mm
sketch2::distance([line3.start, line3.end]) == 6mm
}
"#;

let point_freedoms = run_with_freedom_analysis(kcl).await;

// Expected: line1 has both ends constrained -> Fixed, Fixed
// line2 has one end constrained -> Fixed, Free (but currently shows Fixed, Conflict - bug)
// line3 has conflicting distance constraints -> Conflict, Conflict (but currently shows Free, Free - bug)
// Note: IDs skip every third because segments don't get freedom values
// Format: (ObjectId, Freedom)

println!("Point freedoms: {:?}", point_freedoms);
Copy link
Contributor

Choose a reason for hiding this comment

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

assert_eq!() will print the two things compared when it fails. So you shouldn't need this. If you're asserting the length of an array or something, and you want to print more info, it's generally more idiomatic to include the value in the assertion message of assert_eq!() or assert!(). It's a format string, just like with println, and only prints if the assertion fails.


let expected = vec![
(ObjectId(1), Freedom::Fixed),
(ObjectId(2), Freedom::Fixed),
(ObjectId(4), Freedom::Fixed),
(ObjectId(5), Freedom::Free),
(ObjectId(7), Freedom::Conflict),
(ObjectId(8), Freedom::Conflict),
];

// This assertion will fail until the bug is fixed
assert_eq!(
point_freedoms, expected,
"Point freedoms should match expected values. Current behavior shows bugs with conflicts and reordered lines."
);
}

#[tokio::test(flavor = "multi_thread")]
async fn test_freedom_analysis_without_conflicts() {
let kcl = r#"
@settings(experimentalFeatures = allow)
sketch(on = YZ) {
line1 = sketch2::line(start = [var 2mm, var 8mm], end = [var 5mm, var 7mm])
line1.start.at[0] == 2
line1.start.at[1] == 8
line1.end.at[0] == 5
line1.end.at[1] == 7
line2 = sketch2::line(start = [var 2mm, var 1mm], end = [var -4.75mm, var -0.88mm])
line2.start.at[0] == 2
line2.start.at[1] == 1
line3 = sketch2::line(start = [var -2.591mm, var -7.081mm], end = [var 1.331mm, var -3.979mm])
sketch2::distance([line3.start, line3.end]) == 4mm
}
"#;

let point_freedoms = run_with_freedom_analysis(kcl).await;

// Expected: line1 has both ends constrained -> Fixed, Fixed
// line2 has one end constrained -> Fixed, Free
// line3 has one distance constraint -> Free, Free (both points can move)

println!("Point freedoms: {:?}", point_freedoms);

// Expected: Fixed, Fixed, Fixed, Free, Free, Free
let expected = vec![
(ObjectId(1), Freedom::Fixed),
(ObjectId(2), Freedom::Fixed),
(ObjectId(4), Freedom::Fixed),
(ObjectId(5), Freedom::Free),
(ObjectId(7), Freedom::Free),
(ObjectId(8), Freedom::Free),
];

assert_eq!(point_freedoms, expected, "Point freedoms should match expected values");
}

#[tokio::test(flavor = "multi_thread")]
async fn test_freedom_analysis_reordered_lines() {
let kcl = r#"
@settings(experimentalFeatures = allow)
sketch(on = YZ) {
line1 = sketch2::line(start = [var 2mm, var 8mm], end = [var 5mm, var 7mm])
line1.start.at[0] == 2
line1.start.at[1] == 8
line1.end.at[0] == 5
line1.end.at[1] == 7
line3 = sketch2::line(start = [var -2.591mm, var -7.081mm], end = [var 1.331mm, var -3.979mm])
sketch2::distance([line3.start, line3.end]) == 4mm
sketch2::distance([line3.start, line3.end]) == 6mm
line2 = sketch2::line(start = [var 2mm, var 1mm], end = [var -4.75mm, var -0.88mm])
line2.start.at[0] == 2
line2.start.at[1] == 1
}
"#;

let point_freedoms = run_with_freedom_analysis(kcl).await;

// Expected: line1 has both ends constrained -> Fixed, Fixed
// line3 has conflicting distance constraints -> Conflict, Conflict (but bug shows one Conflict, one Free)
// line2 has one end constrained -> Fixed, Free

println!("Point freedoms: {:?}", point_freedoms);

let expected = vec![
(ObjectId(1), Freedom::Fixed),
(ObjectId(2), Freedom::Fixed),
(ObjectId(4), Freedom::Conflict),
(ObjectId(5), Freedom::Conflict),
(ObjectId(9), Freedom::Fixed),
(ObjectId(10), Freedom::Free),
];

// This assertion will fail until the bug is fixed
assert_eq!(
point_freedoms, expected,
"Point freedoms should match expected values. Current behavior shows bug where line3.end is Free instead of Conflict."
);
}
}
3 changes: 3 additions & 0 deletions rust/kcl-lib/src/execution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ pub(crate) mod cache;
mod cad_op;
mod exec_ast;
pub mod fn_call;
#[cfg(test)]
#[cfg(feature = "artifact-graph")]
mod freedom_analysis_tests;
mod geometry;
mod id_generator;
mod import;
Expand Down
Loading
Loading