-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[Newton]Replaces nucleus_utils with reimplementation as part of IsaacLab #4097
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
base: dev/newton
Are you sure you want to change the base?
[Newton]Replaces nucleus_utils with reimplementation as part of IsaacLab #4097
Conversation
|
Requires python logging PR merge first |
Greptile OverviewGreptile SummaryReplaces external IsaacSim nucleus utilities dependency with internal implementation in Critical Issues:
Additional Changes:
Confidence Score: 0/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Test as Test File
participant NucleusUtils as isaaclab.utils.nucleus
participant CarbSettings as carb.settings
participant OmniClient as omni.client
Test->>NucleusUtils: get_assets_root_path()
NucleusUtils->>CarbSettings: get(DEFAULT_ASSET_ROOT_TIMEOUT_SETTING)
CarbSettings-->>NucleusUtils: timeout value or None
NucleusUtils->>CarbSettings: get(DEFAULT_ASSET_ROOT_PATH_SETTING)
CarbSettings-->>NucleusUtils: default_asset_root
alt skip_check is False
NucleusUtils->>NucleusUtils: check_server(default_asset_root, "/Isaac", timeout)
NucleusUtils->>OmniClient: set_hang_detection_time_ms(20000)
NucleusUtils->>OmniClient: stat(server + path)
OmniClient-->>NucleusUtils: result, _
alt result == Result.OK
NucleusUtils->>NucleusUtils: check_server(default_asset_root, "/NVIDIA", timeout)
NucleusUtils->>OmniClient: stat(server + path)
OmniClient-->>NucleusUtils: result, _
alt result == Result.OK
NucleusUtils-->>Test: default_asset_root
else result != Result.OK
NucleusUtils-->>Test: raise RuntimeError
end
else result != Result.OK
NucleusUtils-->>Test: raise RuntimeError
end
else skip_check is True
NucleusUtils-->>Test: default_asset_root
end
|
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.
5 files reviewed, 4 comments
| from PIL import Image, ImageChops | ||
| from pxr import Gf, UsdGeom | ||
|
|
||
| import isaaclab.sim.utils.nucleus as nucleus_utils |
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.
syntax: incorrect import path - file is at isaaclab/utils/nucleus.py not isaaclab/sim/utils/nucleus.py
| import isaaclab.sim.utils.nucleus as nucleus_utils | |
| import isaaclab.utils.nucleus as nucleus_utils |
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.
looks like the import should indeed by isaaclab.utils.nucleus
| from isaacsim.core.utils.carb import set_carb_setting | ||
| from isaacsim.core.utils.viewports import set_camera_view | ||
|
|
||
| import isaaclab.sim.utils.nucleus as nucleus_utils |
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.
syntax: incorrect import path - file is at isaaclab/utils/nucleus.py not isaaclab/sim/utils/nucleus.py
| import isaaclab.sim.utils.nucleus as nucleus_utils | |
| import isaaclab.utils.nucleus as nucleus_utils |
| from isaacsim.core.prims import Articulation | ||
| from isaacsim.core.utils.carb import set_carb_setting | ||
|
|
||
| import isaaclab.sim.utils.nucleus as nucleus_utils |
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.
syntax: incorrect import path - file is at isaaclab/utils/nucleus.py not isaaclab/sim/utils/nucleus.py
| import isaaclab.sim.utils.nucleus as nucleus_utils | |
| import isaaclab.utils.nucleus as nucleus_utils |
| DEFAULT_ASSET_ROOT_TIMEOUT_SETTING = "/persistent/isaac/asset_root/timeout" | ||
|
|
||
|
|
||
| def check_server(server: str, path: str, timeout: float = 10.0) -> bool: |
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.
logic: timeout parameter is defined but never used in the function
| def check_server(server: str, path: str, timeout: float = 10.0) -> bool: | |
| def check_server(server: str, path: str) -> bool: |
| import logging | ||
|
|
||
| import carb | ||
| import omni.client |
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.
these omni.client imports could also be a problem
| from PIL import Image, ImageChops | ||
| from pxr import Gf, UsdGeom | ||
|
|
||
| import isaaclab.sim.utils.nucleus as nucleus_utils |
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.
looks like the import should indeed by isaaclab.utils.nucleus
Description
Replaces import of
try:
import isaacsim.storage.native as nucleus_utils
except ModuleNotFoundError:
import isaacsim.core.utils.nucleus as nucleus_utils
with own utils implemented inside the sim utils folder.
import isaaclab.sim.utils.nucleus as nucleus_utils
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there