-
Notifications
You must be signed in to change notification settings - Fork 231
Add support for geom contact margin #1253
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
Closed
gyeomannvidia
wants to merge
21
commits into
newton-physics:main
from
gyeomannvidia:dev/gyeoman/geom_margin
Closed
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
aac1204
Add support for geom contact margin
gyeomannvidia 1e1b51b
Add support for geom contact margin
gyeomannvidia 8ab1a14
Merge branch 'dev/gyeoman/geom_margin' of https://github.com/gyeomann…
gyeomannvidia 4819451
uvx pre-commit run -a
gyeomannvidia 8df96f2
uvx pre-commit run -a
gyeomannvidia df44377
Merge branch 'dev/gyeoman/geom_margin' of https://github.com/gyeomann…
gyeomannvidia 4bad7bf
Merge remote-tracking branch 'origin/main' into dev/gyeoman/geom_margin
gyeomannvidia 6925a7d
Removing noqa: PLC0415 comments, as suggested by coderabbit
gyeomannvidia c88d654
It turns out that # noqa: PLC0415 is required to silence a warning fr…
gyeomannvidia 7ba68fb
We can simply set the value as follows because we already test if the…
gyeomannvidia cd9e314
Add comment that margin is not a custom attribute.
gyeomannvidia 9d1764e
Set margin to 0 for mujoco to replicate the previous behaviour
gyeomannvidia 8f6eacf
builder.rigid_contact_margin = 0.0 to match historic behaviour
gyeomannvidia 0cb9483
builder.rigid_contact_margin = 0.0 to match historic behaviour
gyeomannvidia f17bb84
Correct test comment
gyeomannvidia 1822858
Use parse_float
gyeomannvidia ebec7e8
Merge remote-tracking branch 'origin/main' into dev/gyeoman/geom_margin
gyeomannvidia 8062092
Run collision detection every sim step to avoid large contact bias wi…
gyeomannvidia 96f56c9
Remove solver_name
gyeomannvidia 2491652
uvx pre-commit run -a
gyeomannvidia 0688c66
Merge remote-tracking branch 'origin/main' into dev/gyeoman/geom_margin
gyeomannvidia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if that assert is really needed, the attribute should always be there and the test is going to fail anyway if you're going to access it. We never really do this test for other built-ins. But it's a subjective issue, leaving the decision up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's really just a reminder that we are not working with a custom attribute. It's useful for me, even if for nobody else.