Add SmallPTDiff demo - #1877
Draft
a-elrawy wants to merge 3 commits into
Draft
Conversation
Deep-clone CXX construct/temporary expression arguments in StmtClone. Disable restore-tracker reverse_forw inside loops when no tracker is set. Add StmtCloneConstructRepro and ReverseModeLoopPullbackRepro lit tests. Run the loop repro binary so the guard is checked at runtime, not only compile time.
a-elrawy
force-pushed
the
smallpt-diff-demo
branch
from
July 8, 2026 13:32
9c10345 to
19bb9c7
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a differentiable SmallPT (path tracer) demo and a corresponding gradient-validation test suite, alongside compiler/AD infrastructure fixes and regression tests to prevent crashes in statement cloning and reverse-mode loop handling.
Changes:
- Introduces a new differentiable SmallPT demo (
SmallPTDiff.cpp) plus shared radiance/objective/validation helpers used for AD-vs-FD checks. - Adds new regression tests for previously crashing scenarios (
StmtCloneConstructRepro.C,ReverseModeLoopPullbackRepro.C) and a SmallPTDiff validation driver (test/Gradient/SmallPTDiff.C). - Updates Clad internals to deep-clone construct/temporary object expression arguments and adjusts reverse-mode logic around restore-tracker handling in loops.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Misc/RunDemos.C | Adds build/run coverage for the new SmallPTDiff demo and validation binary. |
| test/Gradient/StmtCloneConstructRepro.C | Regression test for CXXTemporaryObjectExpr cloning crash. |
| test/Gradient/SmallPTDiff.C | Test driver that runs AD-vs-FD validation checks for the new demo. |
| test/Gradient/ReverseModeLoopPullbackRepro.C | Regression test for reverse-mode pullback behavior in loops. |
| lib/Differentiator/StmtClone.cpp | Deep-clones args for construct/temporary object expressions to avoid crashes. |
| lib/Differentiator/ReverseModeVisitor.cpp | Adjusts restore-tracker/forward-pass handling when inside loops. |
| demos/ComputerGraphics/smallpt/SmallPTDiffCommon.h | Shared types + custom derivatives for differentiable SmallPT. |
| demos/ComputerGraphics/smallpt/SmallPTDiffRadiance.h | Differentiable radiance/path tracing kernel. |
| demos/ComputerGraphics/smallpt/SmallPTDiffObjectives.h | Differentiable objective functions (pixel/patch/light/mirror). |
| demos/ComputerGraphics/smallpt/SmallPTDiffValidate.h | AD-vs-FD validation helpers and pass/fail printing. |
| demos/ComputerGraphics/smallpt/SmallPTDiff.cpp | Main differentiable SmallPT demo entry point (renders ppm). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a-elrawy
force-pushed
the
smallpt-diff-demo
branch
from
July 8, 2026 13:55
19bb9c7 to
8513ee5
Compare
a-elrawy
force-pushed
the
smallpt-diff-demo
branch
from
July 8, 2026 14:26
8513ee5 to
fa224e0
Compare
a-elrawy
force-pushed
the
smallpt-diff-demo
branch
from
July 8, 2026 14:58
fa224e0 to
ba15dc2
Compare
a-elrawy
force-pushed
the
smallpt-diff-demo
branch
from
July 8, 2026 15:09
ba15dc2 to
a72a2d9
Compare
- SmallPTDiff.cpp and SmallPTDiffCommon/Radiance/Objectives/Validate headers - test/Gradient/SmallPTDiff.C with FD-validated AD objectives - RunDemos.C SmallPTDiff section
a-elrawy
force-pushed
the
smallpt-diff-demo
branch
from
July 8, 2026 15:11
a72a2d9 to
899f28f
Compare
|
|
||
| struct Vec { | ||
| double x, y, z; | ||
| Vec(double x_ = 0, double y_ = 0, double z_ = 0) : x(x_), y(y_), z(z_) {} |
Contributor
There was a problem hiding this comment.
warning: invalid case style for parameter 'x_' [readability-identifier-naming]
Suggested change
| Vec(double x_ = 0, double y_ = 0, double z_ = 0) : x(x_), y(y_), z(z_) {} | |
| Vec(double x = 0, double y_ = 0, double z_ = 0) : x(x), y(y_), z(z_) {} |
|
|
||
| struct Vec { | ||
| double x, y, z; | ||
| Vec(double x_ = 0, double y_ = 0, double z_ = 0) : x(x_), y(y_), z(z_) {} |
Contributor
There was a problem hiding this comment.
warning: invalid case style for parameter 'y_' [readability-identifier-naming]
Suggested change
| Vec(double x_ = 0, double y_ = 0, double z_ = 0) : x(x_), y(y_), z(z_) {} | |
| Vec(double x_ = 0, double y = 0, double z_ = 0) : x(x_), y(y), z(z_) {} |
|
|
||
| struct Vec { | ||
| double x, y, z; | ||
| Vec(double x_ = 0, double y_ = 0, double z_ = 0) : x(x_), y(y_), z(z_) {} |
Contributor
There was a problem hiding this comment.
warning: invalid case style for parameter 'z_' [readability-identifier-naming]
Suggested change
| Vec(double x_ = 0, double y_ = 0, double z_ = 0) : x(x_), y(y_), z(z_) {} | |
| Vec(double x_ = 0, double y_ = 0, double z = 0) : x(x_), y(y_), z(z) {} |
| Vec operator%(const Vec& b) const { | ||
| return Vec(y * b.z - z * b.y, z * b.x - x * b.z, x * b.y - y * b.x); | ||
| } | ||
| Vec mult(const Vec& b) const { return Vec(x * b.x, y * b.y, z * b.z); } |
Contributor
There was a problem hiding this comment.
warning: function 'mult' should be marked [[nodiscard]] [modernize-use-nodiscard]
Suggested change
| Vec mult(const Vec& b) const { return Vec(x * b.x, y * b.y, z * b.z); } | |
| [[nodiscard]] Vec mult(const Vec& b) const { return Vec(x * b.x, y * b.y, z * b.z); } |
| return Vec(y * b.z - z * b.y, z * b.x - x * b.z, x * b.y - y * b.x); | ||
| } | ||
| Vec mult(const Vec& b) const { return Vec(x * b.x, y * b.y, z * b.z); } | ||
| Vec norm() const; |
Contributor
There was a problem hiding this comment.
warning: function 'norm' should be marked [[nodiscard]] [modernize-use-nodiscard]
Suggested change
| Vec norm() const; | |
| [[nodiscard]] Vec norm() const; |
|
|
||
| namespace clad { | ||
| namespace custom_derivatives { | ||
| namespace class_functions { |
Contributor
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
Suggested change
| namespace class_functions { | |
| namespace clad::custom_derivatives::class_functions { |
demos/ComputerGraphics/smallpt/SmallPTDiffCommon.h:79:
- } // namespace class_functions
- } // namespace custom_derivatives
- } // namespace clad
+ } // namespace clad::custom_derivatives::class_functions
+ | } // namespace clad | ||
|
|
||
| non_differentiable inline double sample_erand48(unsigned short* Xi) { | ||
| return erand48(Xi); |
Contributor
There was a problem hiding this comment.
warning: no header providing "erand48" is directly included [misc-include-cleaner]
demos/ComputerGraphics/smallpt/SmallPTDiffCommon.h:7:
+ #include <stdlib.h>| } | ||
|
|
||
| namespace clad { | ||
| namespace custom_derivatives { |
Contributor
There was a problem hiding this comment.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
Suggested change
| namespace custom_derivatives { | |
| namespace clad::custom_derivatives { |
demos/ComputerGraphics/smallpt/SmallPTDiffCommon.h:91:
- } // namespace custom_derivatives
- } // namespace clad
+ } // namespace clad::custom_derivatives|
|
||
| struct Ray { | ||
| Vec o, d; | ||
| Ray() : o(), d() {} |
Contributor
There was a problem hiding this comment.
warning: initializer for member 'd' is redundant [readability-redundant-member-init]
Suggested change
| Ray() : o(), d() {} | |
| Ray() : o(), {} |
|
|
||
| struct Ray { | ||
| Vec o, d; | ||
| Ray() : o(), d() {} |
Contributor
There was a problem hiding this comment.
warning: initializer for member 'o' is redundant [readability-redundant-member-init]
Suggested change
| Ray() : o(), d() {} | |
| Ray() : , d() {} |
a-elrawy
marked this pull request as draft
July 22, 2026 02:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a SmallPTDiff demo with full path-tracing radiance through clad, plus a test validating AD vs FD (camera, patch, light, mirror). Original SmallPT.cpp is unchanged.