refactor(cuda): rewrite revolute joint using F-space Jacobian formulation#404
Conversation
…tion Replace direct world-space energy/gradient/hessian computation with a factored F-space approach: compute constraint violation F, then map gradients and Hessians through the Jacobian (J^T * dEdF, J^T * H * J). Uses DoubletVectorAssembler and TripletMatrixAssembler for cleaner assembly. Regenerate symbolic code from updated notebook. Made-with: Cursor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the revolute joint constraint within the CUDA backend. The core change involves transitioning from a direct world-space calculation to a more robust and modular F-space Jacobian formulation. This approach enhances consistency with other joint implementations, improves the clarity of constraint evaluation, and simplifies the process of extending or verifying the constraint's behavior. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the calculation of affine body revolute joint energy, gradients, and Hessians by transitioning from manual C++ implementations to a symbolic generation approach. A Jupyter notebook (affine_body_revolute_joint.ipynb) now uses SymEigen to generate C++ inline functions (affine_body_revolute_joint.inl) for Faxis, JaxisT_Gaxis, JaxisT_Haxis_Jaxis, Eaxis, dEaxisdFaxis, and ddEaxisddFaxis. The CUDA C++ implementation (affine_body_revolute_joint.cu) has been updated to utilize these new symbolic functions, simplifying the code and ensuring consistency. Feedback includes suggestions to use with open(...) for safer file I/O in the Python script and to replace std::pow(x, 2) with Faxis.squaredNorm() in the generated CUDA code for better performance and conciseness.
| "f = open(path, \"w\")\n", | ||
| "f.write(content)\n", | ||
| "f.close()\n", |
There was a problem hiding this comment.
For file I/O, it's best practice to use a with statement. This ensures the file is automatically and safely closed, even if errors occur. It's more robust and idiomatic Python.
with open(path, "w") as f:
f.write(content)
src/backends/cuda/affine_body/constitutions/sym/affine_body_revolute_joint.inl
Show resolved
Hide resolved
| DVA.segment<StencilSize>(StencilSize * I).write(bids, G24); | ||
|
|
||
| // Fill Body Hessian | ||
| if(!gradient_only) |
There was a problem hiding this comment.
if(gradient_only)
return;
is more readable.
…n up style Add missing gradient_only early return in driving revolute joint computation. Use early-return pattern for gradient_only checks in the axis constraint section. Clean up notebook outputs. Made-with: Cursor
2 participants
Summary
Faxis), with gradients and Hessians mapped back to ABD space throughJ^T * dEdFandJ^T * H * JrespectivelyDoubletVectorAssemblerandTripletMatrixAssemblerfor cleaner gradient/Hessian assembly, consistent with the prismatic joint implementationFaxis,Eaxis,dEaxisdFaxis,ddEaxisddFaxis,JaxisT_Gaxis,JaxisT_Haxis_Jaxis)E()helper from the function header in favor of symbolically generated codeMotivation
The F-space formulation cleanly separates the constraint evaluation from the ABD-to-world mapping, making the code more modular and consistent with the prismatic joint implementation. This factored approach also makes it easier to reason about correctness and extend with additional constraint terms.