Add support for std::initializer_list in the reverse mode #1018

PetroZarytskyi · 2024-08-01T15:19:55Z

This PR adds support for std::initializer_list in the reverse mode.
Also, this PR adds support for non-array type ranges and removes redundant assignments by simplifying the logic for range-based for loops. Apart from that, it adds support for std::begin/std::end, and introduces begin/end functions to clad::array.
Fixes #830.

github-actions

clang-tidy made some suggestions

lib/Differentiator/ReverseModeVisitor.cpp

github-actions · 2024-08-01T16:18:32Z

clang-tidy review says "All clean, LGTM! 👍"

codecov · 2024-08-01T16:22:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.28%. Comparing base (b7f8e5d) to head (d93e52c).
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1018      +/-   ##
==========================================
+ Coverage   94.12%   94.28%   +0.16%     
==========================================
  Files          55       55              
  Lines        8266     8293      +27     
==========================================
+ Hits         7780     7819      +39     
+ Misses        486      474      -12

Files	Coverage Δ
include/clad/Differentiator/EstimationModel.h	`100.00% <ø> (ø)`
include/clad/Differentiator/ReverseModeVisitor.h	`97.97% <ø> (ø)`
include/clad/Differentiator/VisitorBase.h	`100.00% <ø> (ø)`
lib/Differentiator/EstimationModel.cpp	`100.00% <ø> (ø)`
lib/Differentiator/ReverseModeVisitor.cpp	`97.80% <100.00%> (+0.02%)`	⬆️
lib/Differentiator/VisitorBase.cpp	`96.86% <100.00%> (+0.13%)`	⬆️

... and 2 files with indirect coverage changes

Files	Coverage Δ
include/clad/Differentiator/EstimationModel.h	`100.00% <ø> (ø)`
include/clad/Differentiator/ReverseModeVisitor.h	`97.97% <ø> (ø)`
include/clad/Differentiator/VisitorBase.h	`100.00% <ø> (ø)`
lib/Differentiator/EstimationModel.cpp	`100.00% <ø> (ø)`
lib/Differentiator/ReverseModeVisitor.cpp	`97.80% <100.00%> (+0.02%)`	⬆️
lib/Differentiator/VisitorBase.cpp	`96.86% <100.00%> (+0.13%)`	⬆️

... and 2 files with indirect coverage changes

github-actions · 2024-08-05T16:30:19Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions

clang-tidy made some suggestions

lib/Differentiator/VisitorBase.cpp

github-actions · 2024-08-08T13:19:27Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-08-09T17:30:25Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-08-09T18:31:05Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-08-09T18:53:53Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-08-13T11:42:31Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-08-15T11:21:58Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2024-08-15T12:57:21Z

clang-tidy review says "All clean, LGTM! 👍"

lib/Differentiator/ReverseModeVisitor.cpp

vgvassilev

Looks like we are heading in a good direction. See some comments inline.

lib/Differentiator/ReverseModeVisitor.cpp

lib/Differentiator/TBRAnalyzer.cpp

github-actions · 2024-08-19T20:23:47Z

clang-tidy review says "All clean, LGTM! 👍"

lib/Differentiator/ReverseModeVisitor.cpp

vgvassilev · 2024-08-20T13:12:06Z

lib/Differentiator/ReverseModeVisitor.cpp

+            unsigned numInits = ILE->getNumInits();
+            VDDerivedInit = ConstantFolder::synthesizeLiteral(
+                m_Context.getSizeType(), m_Context, numInits);
+            VDCloneType = VDDerivedType;


Why not assigning directly to VDCloneType rather to a temporary VDDerivedType.

VDDerivedType is not temporary, it represents the type of the adjoint variable, whereas VDCloneType represents the type of the cloned variable. The same type is used for both the clone and the adjoint, hence the assignment later.

Ok, I did not realize it was used below. This routine has become quite complicated...

lib/Differentiator/ReverseModeVisitor.cpp

vgvassilev · 2024-08-20T13:19:16Z

lib/Differentiator/ReverseModeVisitor.cpp

+          if (promoteToFnScope) {
+            VDDerivedInit = BuildOp(UO_AddrOf, initDiff.getExpr_dx());
+            VDDerivedType = VDDerivedInit->getType();
+          } else {
+            VDDerivedInit = initDiff.getExpr_dx();
+            VDDerivedType =
+                m_Context.getLValueReferenceType(VDDerivedInit->getType());
+          }
+          VDCloneType = VDDerivedType;


Suggested change

if (promoteToFnScope) {

VDDerivedInit = BuildOp(UO_AddrOf, initDiff.getExpr_dx());

VDDerivedType = VDDerivedInit->getType();

} else {

VDDerivedInit = initDiff.getExpr_dx();

VDDerivedType =

m_Context.getLValueReferenceType(VDDerivedInit->getType());

}

VDCloneType = VDDerivedType;

if (promoteToFnScope) {

VDDerivedInit = BuildOp(UO_AddrOf, initDiff.getExpr_dx());

VDCloneType = VDDerivedInit->getType();

} else {

VDDerivedInit = initDiff.getExpr_dx();

VDCloneType =

m_Context.getLValueReferenceType(VDDerivedInit->getType());

}

This way, VDDerivedType is not modified and the adjoints remain of type std::initializer_list. This breaks the tests for multiple reasons.

vgvassilev · 2024-08-20T13:20:43Z

lib/Differentiator/ReverseModeVisitor.cpp

+          VDCloneType = VDDerivedType;
+        }
+      }
+    }


I believe this block could be further simplified.

github-actions · 2024-08-20T22:47:38Z

clang-tidy review says "All clean, LGTM! 👍"

vgvassilev · 2024-08-21T05:59:42Z

lib/Differentiator/ReverseModeVisitor.cpp

+            unsigned numInits = ILE->getNumInits();
+            VDDerivedInit = ConstantFolder::synthesizeLiteral(
+                m_Context.getSizeType(), m_Context, numInits);
+            VDCloneType = VDDerivedType;


Ok, I did not realize it was used below. This routine has become quite complicated...

lib/Differentiator/ReverseModeVisitor.cpp

…ge-based for loops

Fixes vgvassilev#830.

github-actions · 2024-08-21T15:33:22Z

clang-tidy review says "All clean, LGTM! 👍"

vgvassilev

LGTM!

PetroZarytskyi force-pushed the issue-830 branch 2 times, most recently from 075d682 to 31bd298 Compare August 1, 2024 15:21

github-actions bot reviewed Aug 1, 2024

View reviewed changes

lib/Differentiator/ReverseModeVisitor.cpp Outdated Show resolved Hide resolved

lib/Differentiator/ReverseModeVisitor.cpp Outdated Show resolved Hide resolved

PetroZarytskyi force-pushed the issue-830 branch 2 times, most recently from 46fcab0 to f76201c Compare August 1, 2024 16:11

PetroZarytskyi force-pushed the issue-830 branch from f76201c to d6361cd Compare August 5, 2024 16:22

PetroZarytskyi force-pushed the issue-830 branch 2 times, most recently from b0ea09e to e9e6ed5 Compare August 7, 2024 23:24

github-actions bot reviewed Aug 7, 2024

View reviewed changes

lib/Differentiator/VisitorBase.cpp Outdated Show resolved Hide resolved

lib/Differentiator/VisitorBase.cpp Outdated Show resolved Hide resolved

lib/Differentiator/VisitorBase.cpp Outdated Show resolved Hide resolved

vgvassilev force-pushed the issue-830 branch from e9e6ed5 to d2f5a6c Compare August 8, 2024 06:58

PetroZarytskyi force-pushed the issue-830 branch 3 times, most recently from 8c98120 to 2d982a5 Compare August 8, 2024 13:06

PetroZarytskyi force-pushed the issue-830 branch from 852162e to f572746 Compare August 9, 2024 18:38

PetroZarytskyi force-pushed the issue-830 branch 3 times, most recently from 1952e6b to 09402df Compare August 13, 2024 11:27

PetroZarytskyi force-pushed the issue-830 branch from 09402df to e1e9ac1 Compare August 15, 2024 11:07

PetroZarytskyi force-pushed the issue-830 branch from e1e9ac1 to e2f5ad8 Compare August 15, 2024 12:40

PetroZarytskyi requested a review from vgvassilev August 15, 2024 19:52

vgvassilev reviewed Aug 17, 2024

View reviewed changes

lib/Differentiator/ReverseModeVisitor.cpp Outdated Show resolved Hide resolved

vgvassilev reviewed Aug 17, 2024

View reviewed changes

lib/Differentiator/ReverseModeVisitor.cpp Outdated Show resolved Hide resolved

lib/Differentiator/ReverseModeVisitor.cpp Show resolved Hide resolved

lib/Differentiator/TBRAnalyzer.cpp Outdated Show resolved Hide resolved

PetroZarytskyi mentioned this pull request Aug 19, 2024

Desugar the type before analyzing it in TBR #1046

Merged

PetroZarytskyi force-pushed the issue-830 branch 2 times, most recently from 8f7b4aa to 3a93094 Compare August 19, 2024 20:11

vgvassilev reviewed Aug 20, 2024

View reviewed changes

PetroZarytskyi force-pushed the issue-830 branch from 3a93094 to c5ea72a Compare August 20, 2024 22:35

vgvassilev reviewed Aug 21, 2024

View reviewed changes

PetroZarytskyi added 5 commits August 21, 2024 18:09

Move GetFunctionCall to VisitorBase

dbdb9bd

Add begin/end functions for clad::array

820e9f7

Add a special case to differentiate begin/end calls

640dfa9

Remove redundant assignments and support non-array type ranges in ran…

0189c36

…ge-based for loops

Add support for std::initializer_list in the reverse mode.

0687cee

Fixes vgvassilev#830.

PetroZarytskyi force-pushed the issue-830 branch from c5ea72a to 4cb2d28 Compare August 21, 2024 15:14

Save VD type in a variable in DifferentiateVarDecl

d93e52c

PetroZarytskyi force-pushed the issue-830 branch from 4cb2d28 to d93e52c Compare August 21, 2024 15:20

vgvassilev approved these changes Aug 21, 2024

View reviewed changes

vgvassilev merged commit 66b6af4 into vgvassilev:master Aug 21, 2024
89 checks passed

PetroZarytskyi deleted the issue-830 branch August 21, 2024 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for std::initializer_list in the reverse mode #1018

Add support for std::initializer_list in the reverse mode #1018

PetroZarytskyi commented Aug 1, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot commented Aug 1, 2024

codecov bot commented Aug 1, 2024 •

edited

Loading

github-actions bot commented Aug 5, 2024

github-actions bot left a comment

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

vgvassilev left a comment

github-actions bot commented Aug 19, 2024

vgvassilev Aug 20, 2024

PetroZarytskyi Aug 20, 2024 •

edited

Loading

vgvassilev Aug 21, 2024

vgvassilev Aug 20, 2024

PetroZarytskyi Aug 20, 2024 •

edited

Loading

vgvassilev Aug 20, 2024

github-actions bot commented Aug 20, 2024

vgvassilev Aug 21, 2024

github-actions bot commented Aug 21, 2024

vgvassilev left a comment

Add support for std::initializer_list in the reverse mode #1018

Add support for std::initializer_list in the reverse mode #1018

Conversation

PetroZarytskyi commented Aug 1, 2024 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 1, 2024

codecov bot commented Aug 1, 2024 • edited Loading

Codecov Report

github-actions bot commented Aug 5, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

vgvassilev left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 19, 2024

vgvassilev Aug 20, 2024

Choose a reason for hiding this comment

PetroZarytskyi Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

vgvassilev Aug 21, 2024

Choose a reason for hiding this comment

vgvassilev Aug 20, 2024

Choose a reason for hiding this comment

PetroZarytskyi Aug 20, 2024 • edited Loading

Choose a reason for hiding this comment

vgvassilev Aug 20, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 20, 2024

vgvassilev Aug 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 21, 2024

vgvassilev left a comment

Choose a reason for hiding this comment

PetroZarytskyi commented Aug 1, 2024 •

edited

Loading

codecov bot commented Aug 1, 2024 •

edited

Loading

PetroZarytskyi Aug 20, 2024 •

edited

Loading

PetroZarytskyi Aug 20, 2024 •

edited

Loading