Skip to content

(feature): adds API on Constraint to get var ids #235

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.

Sign up for GitHub

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

Merged
adamchalmers merged 2 commits into from
Mar 31, 2026
Merged

(feature): adds API on Constraint to get var ids #235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1cd9306
(feature): adds API on Constraint to get var ids
therapon Mar 30, 2026
92f1752
adds proptests to enforce invariant between nonzeros and extend_depen…
therapon Mar 31, 2026
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
2 changes: 2 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

* We use `just` to run scripts, so please [install just](https://github.com/casey/just?tab=readme-ov-file#installation)
* This is written in Rust, so please [install Rust via Rustup](https://rustup.rs/)
* ezpz-wasm requires a wasm target installed `rustup target add wasm32-unknown-unknown`
* Install `cargo binstall` to make installing other deps easier: `cargo install cargo-binstall`
* Install `cargo-fuzz` `cargo install cargo-fuzz`
* Then install other tools for development: `cargo binstall cargo-criterion typos-cli cargo-nextest cargo-llvm-cov flamegraph cargo-sort`

If everything was successful, `just check-most` should succeed.
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions ezpz/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ezpz"
version = "0.2.19"
version = "0.2.20"
edition = "2024"
description = "A constraint solver for KCL and Zoo Design Studio"
repository = "https://github.com/KittyCAD/ezpz"
Expand Down Expand Up @@ -68,4 +68,3 @@ doc_markdown = "warn"
struct_field_names = "warn"
trivially_copy_pass_by_ref = "warn"
elidable_lifetime_names = "warn"

246 changes: 245 additions & 1 deletion ezpz/src/constraints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,178 @@ impl std::fmt::Debug for JacobianVar {
}

impl Constraint {
/// Extend `out` with the primitive variable IDs that this constraint's
/// residual equations depend on.
///
/// "Dependent" means changing one of the emitted IDs can change this
/// constraint's residual value. This is intentionally narrower than
/// [`Constraint::extend_associated_variable_ids`], which reports every ID
/// structurally present inside the attached geometry. For example,
/// [`Constraint::HorizontalDistance`] only emits the two X-component IDs
/// because its residual does not depend on either point's Y component, and
/// [`Constraint::CircleRadius`] only emits the radius ID.
///
/// The output collection is owned by the caller so this API does not
/// allocate. Callers that need deduplication can pass a set-like type.
pub fn extend_dependent_variable_ids(&self, out: &mut impl Extend<Id>) {
match self {
Constraint::LineTangentToCircle(line, circle) => {
out.extend(line.all_variables());
Copy link
Copy Markdown
Collaborator

@adamchalmers adamchalmers Mar 31, 2026

Choose a reason for hiding this comment

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

Is this basically going to be computing the exact same data as Constraint::nonzeroes, but collecting into a single collection rather than multiple (row0, row1, row2 etc)?

If so, let's either
A. deduplicate them, so there's only one place that says which IDs can alter the residual
B. Have a property-based test (we use proptest elsewhere in this codebase) that asserts the invariant that extend_dependent_variable_ids should return the same IDs as nonzeroes for any constraint

Copy link
Copy Markdown
Contributor Author

@therapon therapon Mar 31, 2026

Choose a reason for hiding this comment

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

I am going with B for now and leaving A as a follow up.

For A the the refactor would need an internal helper function that both public APIs will call. I'll make an issue for it.

Copy link
Copy Markdown
Contributor Author

@therapon therapon Mar 31, 2026

Choose a reason for hiding this comment

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

See #236

out.extend(circle.all_variables());
}
Constraint::CircleTangentToCircle(circle0, circle1) => {
out.extend(circle0.all_variables());
out.extend(circle1.all_variables());
}
Constraint::Distance(p0, p1, _dist) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
}
Constraint::DistanceVar(p0, p1, d) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
out.extend(d.all_variables());
}
Constraint::VerticalDistance(p0, p1, _dist) => out.extend([p0.id_y(), p1.id_y()]),
Constraint::HorizontalDistance(p0, p1, _dist) => out.extend([p0.id_x(), p1.id_x()]),
Constraint::Vertical(line) => out.extend([line.p0.id_x(), line.p1.id_x()]),
Constraint::Horizontal(line) => out.extend([line.p0.id_y(), line.p1.id_y()]),
Constraint::LinesAtAngle(line0, line1, _angle) => {
out.extend(line0.all_variables());
out.extend(line1.all_variables());
}
Constraint::Fixed(id, _scalar) => out.extend([*id]),
Constraint::ScalarEqual(x, y) => out.extend([*x, *y]),
Constraint::PointsCoincident(p0, p1) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
}
Constraint::CircleRadius(circle, _radius) => out.extend([circle.radius.id]),
Constraint::LinesEqualLength(line0, line1) => {
out.extend(line0.all_variables());
out.extend(line1.all_variables());
}
Constraint::ArcRadius(arc, _radius) => out.extend(arc.all_variables()),
Constraint::Arc(arc) => out.extend(arc.all_variables()),
Constraint::Midpoint(line, point) => {
out.extend([line.p0.id_x(), line.p1.id_x(), point.id_x()]);
out.extend([line.p0.id_y(), line.p1.id_y(), point.id_y()]);
}
Constraint::PointLineDistance(point, line, _distance) => {
out.extend(point.all_variables());
out.extend(line.all_variables());
}
Constraint::VerticalPointLineDistance(point, line, _distance) => {
out.extend(line.all_variables());
out.extend(point.all_variables());
}
Constraint::HorizontalPointLineDistance(point, line, _distance) => {
out.extend(line.all_variables());
out.extend(point.all_variables());
}
Constraint::Symmetric(line, a, b) => {
out.extend(line.all_variables());
out.extend(a.all_variables());
out.extend(b.all_variables());
}
Constraint::PointArcCoincident(circular_arc, point) => {
out.extend(circular_arc.all_variables());
out.extend(point.all_variables());
}
Constraint::ArcLength(circular_arc, _dist) => out.extend(circular_arc.all_variables()),
Constraint::ArcAngle(circular_arc, _angle) => out.extend(circular_arc.all_variables()),
}
}

/// Extend `out` with the primitive variable IDs associated with the
/// geometry attached to this constraint.
///
/// "Associated" means every variable ID belonging to the datums mentioned
/// by this constraint, even if some of those IDs do not affect the residual
/// directly. This is intentionally broader than
/// [`Constraint::extend_dependent_variable_ids`], which only reports IDs
/// that can change the residual itself. For example,
/// [`Constraint::HorizontalDistance`] emits both points' X and Y IDs
/// because both full points are associated with the constraint, and
/// [`Constraint::CircleRadius`] emits the circle center IDs as well as the
/// radius ID because all three belong to the associated circle datum.
///
/// The output collection is owned by the caller so this API does not
/// allocate. Callers that need deduplication can pass a set-like type.
pub fn extend_associated_variable_ids(&self, out: &mut impl Extend<Id>) {
match self {
Constraint::LineTangentToCircle(line, circle) => {
out.extend(line.all_variables());
out.extend(circle.all_variables());
}
Constraint::CircleTangentToCircle(circle0, circle1) => {
out.extend(circle0.all_variables());
out.extend(circle1.all_variables());
}
Constraint::Distance(p0, p1, _dist) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
}
Constraint::DistanceVar(p0, p1, d) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
out.extend(d.all_variables());
}
Constraint::VerticalDistance(p0, p1, _dist)
| Constraint::HorizontalDistance(p0, p1, _dist) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
}
Constraint::Vertical(line) | Constraint::Horizontal(line) => {
out.extend(line.all_variables());
}
Constraint::LinesAtAngle(line0, line1, _angle) => {
out.extend(line0.all_variables());
out.extend(line1.all_variables());
}
Constraint::Fixed(id, _scalar) => out.extend([*id]),
Constraint::ScalarEqual(x, y) => out.extend([*x, *y]),
Constraint::PointsCoincident(p0, p1) => {
out.extend(p0.all_variables());
out.extend(p1.all_variables());
}
Constraint::CircleRadius(circle, _radius) => out.extend(circle.all_variables()),
Constraint::LinesEqualLength(line0, line1) => {
out.extend(line0.all_variables());
out.extend(line1.all_variables());
}
Constraint::ArcRadius(arc, _radius) => out.extend(arc.all_variables()),
Constraint::Arc(arc) => out.extend(arc.all_variables()),
Constraint::Midpoint(line, point) => {
out.extend(line.all_variables());
out.extend(point.all_variables());
}
Constraint::PointLineDistance(point, line, _distance) => {
out.extend(point.all_variables());
out.extend(line.all_variables());
}
Constraint::VerticalPointLineDistance(point, line, _distance) => {
out.extend(line.all_variables());
out.extend(point.all_variables());
}
Constraint::HorizontalPointLineDistance(point, line, _distance) => {
out.extend(line.all_variables());
out.extend(point.all_variables());
}
Constraint::Symmetric(line, a, b) => {
out.extend(line.all_variables());
out.extend(a.all_variables());
out.extend(b.all_variables());
}
Constraint::PointArcCoincident(circular_arc, point) => {
out.extend(circular_arc.all_variables());
out.extend(point.all_variables());
}
Constraint::ArcLength(circular_arc, _dist) => out.extend(circular_arc.all_variables()),
Constraint::ArcAngle(circular_arc, _angle) => out.extend(circular_arc.all_variables()),
}
}

/// For each row of the Jacobian matrix, which variables are involved in them?
pub(crate) fn nonzeroes(&self, row0: &mut Vec<Id>, row1: &mut Vec<Id>, _row2: &mut Vec<Id>) {
match self {
Expand Down Expand Up @@ -2352,10 +2524,82 @@ fn wrap_angle_delta(delta: f64) -> f64 {
mod tests {
use std::f64::consts::SQRT_2;

use crate::tests::assert_nearly_eq;
use crate::{
IdGenerator,
datatypes::inputs::{
DatumCircle, DatumCircularArc, DatumDistance, DatumLineSegment, DatumPoint,
},
tests::assert_nearly_eq,
};

use super::*;

#[test]
fn extend_dependent_variable_ids_reports_only_referenced_components() {
let mut ids = IdGenerator::default();
let p0 = DatumPoint::new(&mut ids);
let p1 = DatumPoint::new(&mut ids);

let horizontal = Constraint::HorizontalDistance(p0, p1, 10.0);
let mut horizontal_ids = Vec::with_capacity(2);
horizontal.extend_dependent_variable_ids(&mut horizontal_ids);
assert_eq!(horizontal_ids, vec![p0.id_x(), p1.id_x()]);

let vertical = Constraint::Vertical(DatumLineSegment::new(p0, p1));
let mut vertical_ids = Vec::with_capacity(2);
vertical.extend_dependent_variable_ids(&mut vertical_ids);
assert_eq!(vertical_ids, vec![p0.id_x(), p1.id_x()]);
}

#[test]
fn extend_associated_variable_ids_reports_all_datum_components() {
let mut ids = IdGenerator::default();
let p0 = DatumPoint::new(&mut ids);
let p1 = DatumPoint::new(&mut ids);

let horizontal = Constraint::HorizontalDistance(p0, p1, 10.0);
let mut horizontal_ids = Vec::with_capacity(4);
horizontal.extend_associated_variable_ids(&mut horizontal_ids);
assert_eq!(
horizontal_ids,
vec![p0.id_x(), p0.id_y(), p1.id_x(), p1.id_y()]
);

let circle = DatumCircle {
center: p0,
radius: DatumDistance::new(ids.next_id()),
};
let mut circle_ids = Vec::with_capacity(3);
Constraint::CircleRadius(circle, 5.0).extend_associated_variable_ids(&mut circle_ids);
assert_eq!(
circle_ids,
vec![circle.center.id_x(), circle.center.id_y(), circle.radius.id]
);
}

#[test]
fn associated_and_dependent_variable_id_methods_accept_set_outputs() {
let mut ids = IdGenerator::default();
let arc = DatumCircularArc {
center: DatumPoint::new(&mut ids),
start: DatumPoint::new(&mut ids),
end: DatumPoint::new(&mut ids),
};
let constraint = Constraint::ArcRadius(arc, 5.0);

let mut out = std::collections::HashSet::new();
constraint.extend_dependent_variable_ids(&mut out);
constraint.extend_associated_variable_ids(&mut out);

assert_eq!(out.len(), 6);
assert!(out.contains(&arc.center.id_x()));
assert!(out.contains(&arc.center.id_y()));
assert!(out.contains(&arc.start.id_x()));
assert!(out.contains(&arc.start.id_y()));
assert!(out.contains(&arc.end.id_x()));
assert!(out.contains(&arc.end.id_y()));
}

#[test]
fn test_pds_of_symmetric() {
// Arbitrarily chosen values.
Expand Down
Loading
Loading