Skip to content

Sequential Box<>Box #208

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

Merged
merged 16 commits into from
May 12, 2025
Merged

Conversation

camevor
Copy link
Collaborator

@camevor camevor commented Apr 30, 2025

This is a reimplementation of box<>box, based on the mjc code rather than the mjx code. It is sequential and does not make use of tiles, but closer to mjc and may lead to lower thread divergence.

It fixes issues observed in #163

@camevor camevor marked this pull request as ready for review May 7, 2025 12:15
Copy link
Collaborator

@erikfrey erikfrey left a comment

Choose a reason for hiding this comment

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

This looks good! Just one nit and we'll merge.

@@ -1718,6 +1730,497 @@ def capsule_box(
)


@wp.func
def compute_rotmore(face_idx: int) -> wp.mat33:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: let's prepend the name with an underscore as we probably don't expect others to use this outside this module, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, changed the name to _compute_rotmore.

@thowell thowell merged commit 406d46b into google-deepmind:main May 12, 2025
5 checks passed
@camevor camevor deleted the box-box-sequential branch May 13, 2025 08:23
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.

3 participants