Runtime switch to read transforms from GPU.#5411
Conversation
Greptile SummaryThis PR replaces the hardcoded Confidence Score: 4/5Safe to merge; the change is a small, well-scoped version guard with only minor P2 concerns. Only P2 findings: a possible off-by-one on the version boundary (> vs >=) and the absence of a try/except around the module-level version parse. Neither is a runtime blocker under normal conditions, but both could surprise future maintainers or users on edge-case ovrtx builds. source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py – verify the exact ovrtx release that fixed the GPU-transform bug to confirm the > boundary. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Module import] --> B{ovrtx.__version__ > 0.2.0?}
B -- Yes --> C[_OVRTX_READ_GPU_TRANSFORMS = True]
B -- No --> D[_OVRTX_READ_GPU_TRANSFORMS = False]
C --> E[OVRTXRenderer.initialize]
D --> E
E --> F["RendererConfig(read_gpu_transforms=_OVRTX_READ_GPU_TRANSFORMS)"]
F --> G[Renderer created]
Reviews (1): Last reviewed commit: "Runtime switch to read transforms from G..." | Re-trigger Greptile |
|
|
||
| # In previous versions of ovrtx, there was a bug where we would have to set read_gpu_transforms to False. | ||
| # In later versions, we can read transforms from GPU. | ||
| _OVRTX_READ_GPU_TRANSFORMS = Version(ovrtx.__version__) > Version("0.2.0") |
There was a problem hiding this comment.
Version boundary may be off-by-one
The comment states the bug existed in "previous versions," but the threshold > Version("0.2.0") treats 0.2.0 itself as a buggy version (leaves read_gpu_transforms=False). If the GPU-transform fix was introduced in 0.2.0 (rather than strictly after it), the condition should be >= Version("0.2.0"). Worth confirming the exact release that introduced the fix.
|
|
||
| # In previous versions of ovrtx, there was a bug where we would have to set read_gpu_transforms to False. | ||
| # In later versions, we can read transforms from GPU. | ||
| _OVRTX_READ_GPU_TRANSFORMS = Version(ovrtx.__version__) > Version("0.2.0") |
There was a problem hiding this comment.
No guard against malformed or missing
__version__
Version(ovrtx.__version__) is evaluated at module-import time. If ovrtx doesn't expose __version__ (AttributeError) or its value isn't a valid PEP 440 string (packaging.version.InvalidVersion), the entire module fails to import. Consider wrapping this in a try/except so a version-parsing failure degrades gracefully (e.g. falls back to False):
try:
_OVRTX_READ_GPU_TRANSFORMS = Version(ovrtx.__version__) > Version("0.2.0")
except Exception:
_OVRTX_READ_GPU_TRANSFORMS = False02c493f to
5fd65db
Compare
# Description Runtime check to turn on reading transforms from GPU. Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. Fixes # (issue) <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (existing functionality will not work without user modification) - Documentation update ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task -->
Description
Runtime check to turn on reading transforms from GPU.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there