Skip to content

Add rendering functionals#1618

Open
loliverhennigh wants to merge 18 commits into
NVIDIA:mainfrom
loliverhennigh:feature/rendering-functionals
Open

Add rendering functionals#1618
loliverhennigh wants to merge 18 commits into
NVIDIA:mainfrom
loliverhennigh:feature/rendering-functionals

Conversation

@loliverhennigh

Copy link
Copy Markdown
Collaborator

PhysicsNeMo Pull Request

Description

Rendering Kernels

@copy-pr-bot

copy-pr-bot Bot commented May 5, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps

greptile-apps Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a new physicsnemo.nn.functional.rendering package with eight GPU-accelerated rendering primitives (isosurface ray-marching, volume rendering, mesh ray-casting, LIC, point cloud, wireframe, and scalar/vector-field RGBA transfer functions) backed by custom Warp kernels and optional PyTorch fallbacks. The FunctionSpec dispatch pattern and test coverage are solid, but several correctness and portability issues in the core Warp implementation file need attention before merging.

  • TOCTOU race in wireframe rasterization_write_depth_tested_pixel uses wp.atomic_min for depth but writes color non-atomically; two threads at similar depths can both pass the depth test and write their colors in an unordered way, producing incorrect results.
  • wp.init() at module import time — calling Warp initialization unconditionally at the top of _warp_impl.py causes every import of the rendering package to initialize the GPU, breaking CPU-only and Warp-free environments even when only the PyTorch backends are needed.
  • GPU validation gaps — camera and bounds sanity checks in _camera_basis and _bounds_tensor are skipped on CUDA devices, so degenerate inputs (eye == center, up parallel to view, invalid bounds) silently produce corrupted renders on GPU.

Important Files Changed

Filename Overview
physicsnemo/nn/functional/rendering/_warp_impl.py Core Warp kernel file; contains a TOCTOU race in _write_depth_tested_pixel, module-level wp.init() at import time, GPU-only validation gaps in _camera_basis/_bounds_tensor, a hardcoded 8192-step cap for line rasterization, and no near-plane clipping for wireframe edges.
physicsnemo/nn/functional/rendering/_torch_impl.py Pure-PyTorch fallback implementations for scalar_field_to_rgba and vector_field_to_rgba; logic is clean and correct.
physicsnemo/nn/functional/rendering/isosurface_render.py FunctionSpec wrapper for isosurface ray-marching; delegates to warp backend, validation and make_inputs_forward look correct.
physicsnemo/nn/functional/rendering/mesh_raycast.py FunctionSpec wrapper for BVH mesh ray-casting; input validation and dispatch are correct.
physicsnemo/nn/functional/rendering/wireframe_render.py Thin FunctionSpec wrapper for wireframe rasterization; the underlying kernel issues are in _warp_impl.py.
physicsnemo/nn/functional/rendering/point_cloud_render.py FunctionSpec wrapper for two-pass point cloud rasterization; uses race-safe atomic key scheme unlike wireframe.
physicsnemo/nn/functional/rendering/volume_render.py VolumeRender FunctionSpec wrapper; front-to-back compositing logic is correct.
physicsnemo/nn/functional/rendering/scalar_field_to_rgba.py ScalarFieldToRGBA with warp and torch backends; compare_forward tolerance looks appropriate.
physicsnemo/nn/functional/rendering/vector_field_to_rgba.py VectorFieldToRGBA with warp and torch backends; logic mirrors scalar_field_to_rgba correctly.
physicsnemo/nn/functional/rendering/line_integral_convolution.py LineIntegralConvolution FunctionSpec wrapper; delegates to warp kernel, looks correct.
physicsnemo/nn/functional/rendering/init.py Exports all rendering classes and functional aliases; all is complete and consistent.

Reviews (1): Last reviewed commit: "Add rendering functionals" | Re-trigger Greptile

Comment thread physicsnemo/nn/functional/rendering/_warp_ops.py Outdated
Comment thread physicsnemo/nn/functional/rendering/_warp_ops.py Outdated
Comment thread physicsnemo/nn/functional/rendering/_warp_ops.py Outdated
Comment thread physicsnemo/nn/functional/rendering/_warp_ops.py Outdated
Comment thread physicsnemo/nn/functional/rendering/_warp_ops.py Outdated
@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

1 similar comment
@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@megnvidia megnvidia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@mehdiataei

Copy link
Copy Markdown
Collaborator

Amazing! LGTM!

@mehdiataei

Copy link
Copy Markdown
Collaborator

/ok to test ebabe20

@copy-pr-bot

copy-pr-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown

/ok to test ebabe20

@mehdiataei, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

@mehdiataei

Copy link
Copy Markdown
Collaborator

/ok to test 31f6ed6

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

All visible CI is now green on the latest head (pre-commit, Test with uv, and blossom-ci). @ktangsali, could you please take a final review pass when you have a chance?

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

1 similar comment
@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@ktangsali ktangsali left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Do we plan to include these functionals to the ASV functional registry?

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/ok to test 4f4b2ea

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/blossom-ci

@loliverhennigh

Copy link
Copy Markdown
Collaborator Author

/ok to test f588713

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.

4 participants