Skip to content
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.

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

Don't infer redundant constraints #302

Closed
azonenberg opened this issue Nov 8, 2017 · 7 comments
Closed

Don't infer redundant constraints #302

azonenberg opened this issue Nov 8, 2017 · 7 comments
Milestone

Comments

@azonenberg
Copy link

Horizontal and vertical constraints are inferred automatically even if this would result in a redundant situation. When sketching on top of existing geometry, this results in an unsatisfiable situation after adding most geometry.

For example: when a new vertical line is added right on top of an existing vertical line from previous geometry, it's constrained to be on the existing line but also to be vertical. This causes the system to be unsatisfiable as soon as it's drawn.

This may be a more fundamental question about how we want to handle redundant constraints. Although both the two "point on curve" and the "vertical" constraints are unnecessary (any two of the three is sufficient to constrain the new line), the redundant constraint is consistent with the existing ones. Why does this result in an unsatisfiable situation instead of working as expected?

@whitequark whitequark changed the title Enhancement: Don't infer redundant constraints Don't infer redundant constraints Nov 8, 2017
@baryluk
Copy link

baryluk commented May 29, 2018

I kind of agree, but this is partially due to accuracy of number used and the fact that coordinates of all points are independent in the solver, so it is hard to detect if the constraints are really redundant or just very close to each other, but conflicting, and unsolvable. Some very smart hybrid (algebraic + numeric) solver could detect most of the common redundancies and eliminate them, point out which ones redundant, or just continue to work without problem. That would be kind of nice, but that can lead people to adding way too many constraints, and not think what they are doing. I know it is currently very normal to deal with redundant or unsatisfied systems all the time, and it is kind of a game with solvespace we pay for it being nice parametric system. I have no idea how other parametric modeling CADs deal with it.

@Evil-Spirit
Copy link
Collaborator

Evil-Spirit commented May 31, 2018

No problem to analyze redundant constraints. Actually, we already used same approach while we adding new constraints with dimension values. If they cause overdefinition of equation system we just mark them as reference automatically after addition. The problem is what we have to avoid using solving by substitution during process of redundant analysis, so this can cause performance issues. But now we have pending improvements about performance and timeouting redundant constraints computations. @whitequark can answer the question about when we can merge this new imorovements.

@whitequark
Copy link
Contributor

@whitequark can answer the question about when we can merge this new imorovements.

The answer is: now! :)

@whitequark whitequark added this to the 3.0 milestone May 24, 2019
@whitequark
Copy link
Contributor

Fixed in 5495659.

@whitequark
Copy link
Contributor

@Evil-Spirit I've also added this logic to recognize newly added redundant constraints based on rank, what do you think about it? cf2f0e5

@baryluk
Copy link

baryluk commented Jan 16, 2020

Just a bit of a report, I really like this new feature, and it works really well. It reduces number of time I get over constrainted sketches probably by a factor of 3. :)

Thanks.

@Evil-Spirit
Copy link
Collaborator

@whitequark good thing )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants