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

Remove object references cycle in LinkDescription #374

Merged
merged 2 commits into from
Feb 18, 2025
Merged

Conversation

younik
Copy link
Member

@younik younik commented Feb 17, 2025

Currently, LinkDescription has the attributes parent and children that are LinkDescription. This creates a reference where a LinkDescription references its parent, which references back to the LinkDescription.

This can create infinite recursion in case we call a function on the attributes of LinkDescription recursively, for example in __repr__, __hash__ or dataclasses.asdict.

The last one is needed to implement a Gymnax environment, which I believe is a common use case.

In this PR, I changed the parent attribute to be the parent's name, instead of the object itself. This doesn't create a problem since most of the time we only need the name and there is a dictionary mapping names to LinkDescription.

To reproduce the issue:

from dataclasses import asdict
import jaxsim.api as js

model = js.model.JaxSimModel.build_from_model_description(
    model_description=...,
    is_urdf=True,
)
asdict(model)

📚 Documentation preview 📚: https://jaxsim--374.org.readthedocs.build//374/

@younik younik self-assigned this Feb 17, 2025
Copy link
Collaborator

@flferretti flferretti left a comment

Choose a reason for hiding this comment

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

Thanks a lot @younik, this is very useful also for #351 and for @xela-95! I just added a couple minor comments

Copy link
Member

@xela-95 xela-95 left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

@flferretti flferretti merged commit a4a7153 into main Feb 18, 2025
18 of 24 checks passed
@flferretti flferretti deleted the fix-cycles branch February 18, 2025 14:51
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