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

Improve constraint violation visualization #125

Merged
merged 2 commits into from
Mar 20, 2024

Conversation

tjstienstra
Copy link
Contributor

@tjstienstra tjstienstra commented Feb 14, 2024

Fixes #123 and #53

Changes

Correct the slicing of the constraints_violation vector.
The constraint vector is formatted as (refer to the creation of the constraint function and the wrapper:

[con_1_2, ..., con_1_N,
con_2_2, ..., con_2_N,
...,
con_n_2, ..., con_n_N,
*instance_constraints]

where con_1_2, ..., con_1_N the constraint violation is at every node for the first discrete equation of motion.

Only create a single axis for showing the EoM violation, as they are not officially coupled to specific states. Yes, it is correct to say that most will specify the first n//2 as kinematic differential equations, but that is not required.

Only plot the constraint violations of the instance constraints if there are any instance constraints.

@tjstienstra tjstienstra marked this pull request as draft February 14, 2024 15:00
@tjstienstra
Copy link
Contributor Author

Here is the new version of the pendulum from the examples page
constr_viols_pend
And here are the plot constraints of the first optimization problem in the BRiM paper
constr_viols

@tjstienstra tjstienstra marked this pull request as ready for review February 14, 2024 15:38
@moorepants
Copy link
Member

Is setting the x tick labels at 90 degree rotation better, in general? If so, maybe adjust that while you are at it.

@tjstienstra
Copy link
Contributor Author

Is setting the x tick labels at 90 degree rotation better, in general? If so, maybe adjust that while you are at it.

I don't like those ticks in general, they always make a mess especially if a constraint is a longer equation. In this case, if you would choose -90, you actually run into the problem that a part of your equation falls outside (below) the figure.

@tjstienstra
Copy link
Contributor Author

It is that you like to see which one it approximately is, but otherwise I would just throw them all together in a single boxplot

@moorepants
Copy link
Member

We can go with this as is and address formatting later.

@moorepants
Copy link
Member

Forgot about this. Merging.

@moorepants moorepants merged commit 599d874 into csu-hmc:master Mar 20, 2024
12 checks passed
@moorepants
Copy link
Member

The docs show this for the pendulum swing up:
pendulum_swing_up_01_new
which is quite smooth compared to the old:
pendulum_swing_up_01

@moorepants
Copy link
Member

Also isn't as small.

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

Successfully merging this pull request may close these issues.

Problem with: prob.plot_constraint_violations(solution)
2 participants