(feature): adds API on Constraint to get var ids#235
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #235 +/- ##
==========================================
- Coverage 90.86% 90.37% -0.50%
==========================================
Files 23 23
Lines 5025 5215 +190
==========================================
+ Hits 4566 4713 +147
- Misses 459 502 +43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance
Comparing Footnotes
|
| pub fn extend_dependent_variable_ids(&self, out: &mut impl Extend<Id>) { | ||
| match self { | ||
| Constraint::LineTangentToCircle(line, circle) => { | ||
| out.extend(line.all_variables()); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Adds two new methods on
Constraintthat provide the variables IDs for a constraintextend_dependent_variable_ids(self, Extend<Id>). Extends the passed collection to include variable Ids that change the constraint's residual values. Var Ids that affect the constraint only.extend_associated_variable_ids(self, Extend<Id>). Extends the passed collection to include variable Ids associated with the constraint--both those that affect the residual and those that do not.Closes: #234