Add support for call expressions in vector forward mode AD #638

vaithak · 2023-10-08T17:10:06Z

Two main changes in this PR:

Renamed ForwardModeVisitor to a more apt name PushForwardModeVisitor.

Made changes to make the VisitCallExpr of BaseForwardModeVistor and the PushForwardModeVisitor class generic enough to add support for vector mode with very little code repetition,

This led to the following inheritance structure,

                                BaseForwardModeVisitor
                              /                        \
                             /                          \
      VectorForwardModeVisitor                          PushForwardModeVisitor
                  |               
                  |                         
    VectorPushForwardModeVisitor

Also added appropriate tests for the new functionality.

Edited: Modified the inheritance diagram as per updated changes.

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 19. Check the log or trigger a new build to see more.

include/clad/Differentiator/VectorPushForwardModeVisitor.h

lib/Differentiator/PushForwardModeVisitor.cpp

github-actions

clang-tidy made some suggestions

lib/Differentiator/PushForwardModeVisitor.cpp

lib/Differentiator/VectorPushForwardModeVisitor.cpp

codecov · 2023-10-10T09:37:27Z

Codecov Report

Merging #638 (c0db619) into master (0ab0c6e) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #638      +/-   ##
==========================================
+ Coverage   94.30%   94.36%   +0.06%     
==========================================
  Files          45       48       +3     
  Lines        7003     7063      +60     
==========================================
+ Hits         6604     6665      +61     
+ Misses        399      398       -1

Files	Coverage Δ
...clude/clad/Differentiator/BaseForwardModeVisitor.h	`100.00% <ø> (ø)`
include/clad/Differentiator/DerivativeBuilder.h	`100.00% <ø> (ø)`
...clude/clad/Differentiator/PushForwardModeVisitor.h	`100.00% <100.00%> (ø)`
...clad/Differentiator/VectorPushForwardModeVisitor.h	`100.00% <100.00%> (ø)`
lib/Differentiator/BaseForwardModeVisitor.cpp	`98.84% <100.00%> (+0.11%)`	⬆️
lib/Differentiator/DerivativeBuilder.cpp	`100.00% <100.00%> (ø)`
lib/Differentiator/PushForwardModeVisitor.cpp	`100.00% <100.00%> (ø)`
lib/Differentiator/VectorForwardModeVisitor.cpp	`100.00% <100.00%> (ø)`
...ib/Differentiator/VectorPushForwardModeVisitor.cpp	`100.00% <100.00%> (ø)`

Files	Coverage Δ
...clude/clad/Differentiator/BaseForwardModeVisitor.h	`100.00% <ø> (ø)`
include/clad/Differentiator/DerivativeBuilder.h	`100.00% <ø> (ø)`
...clude/clad/Differentiator/PushForwardModeVisitor.h	`100.00% <100.00%> (ø)`
...clad/Differentiator/VectorPushForwardModeVisitor.h	`100.00% <100.00%> (ø)`
lib/Differentiator/BaseForwardModeVisitor.cpp	`98.84% <100.00%> (+0.11%)`	⬆️
lib/Differentiator/DerivativeBuilder.cpp	`100.00% <100.00%> (ø)`
lib/Differentiator/PushForwardModeVisitor.cpp	`100.00% <100.00%> (ø)`
lib/Differentiator/VectorForwardModeVisitor.cpp	`100.00% <100.00%> (ø)`
...ib/Differentiator/VectorPushForwardModeVisitor.cpp	`100.00% <100.00%> (ø)`

parth-07 · 2023-10-27T23:56:03Z

This change looks good overall. I would like to avoid diamond hierarchy if possible, but I am not sure if it would be. So many things can easily go wrong when diamon hierarchy is used.

vaithak · 2023-11-04T20:55:56Z

Fixed the inheritance structure to remove the diamond problem, now, the structure is pretty simple:

                                     BaseForwardModeVisitor
                                   /                        \
                                  /                          \
           VectorForwardModeVisitor                          PushForwardModeVisitor
                       |               
                       |                         
         VectorPushForwardModeVisitor

The common code in both PushForwardMode modes is pushed into BaseForwardMode class, with the differences shifted inside separate overridable methods.

Closed by mistake, reopened.

github-actions

clang-tidy made some suggestions

include/clad/Differentiator/PushForwardModeVisitor.h

include/clad/Differentiator/VectorForwardModeVisitor.h

github-actions · 2023-11-04T21:16:31Z

include/clad/Differentiator/VectorPushForwardModeVisitor.h

+#include "VectorForwardModeVisitor.h"
+
+namespace clad {
+class VectorPushForwardModeVisitor : public VectorForwardModeVisitor {


warning: class 'VectorPushForwardModeVisitor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class VectorPushForwardModeVisitor : public VectorForwardModeVisitor { ^

github-actions · 2023-11-04T21:16:32Z

lib/Differentiator/BaseForwardModeVisitor.cpp

+      returnType, paramTypes, originalFnType->getExtProtoInfo());
+  llvm::SaveAndRestore<DeclContext*> saveContext(m_Sema.CurContext);
+  llvm::SaveAndRestore<Scope*> saveScope(m_CurScope);
+  auto* DC = const_cast<DeclContext*>(m_Function->getDeclContext());


warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]

auto* DC = const_cast<DeclContext*>(m_Function->getDeclContext()); ^

github-actions

clang-tidy made some suggestions

github-actions · 2023-11-04T22:04:21Z

include/clad/Differentiator/PushForwardModeVisitor.h

+namespace clad {
+/// A visitor for processing the function code in forward mode.
+/// Used to compute derivatives by clad::differentiate.
+class PushForwardModeVisitor : public BaseForwardModeVisitor {


warning: class 'PushForwardModeVisitor' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class PushForwardModeVisitor : public BaseForwardModeVisitor { ^

vgvassilev · 2023-12-01T17:49:37Z

@vaithak, can you rebase this PR on top of master?

vgvassilev

LGTM!

github-actions bot reviewed Oct 8, 2023

View reviewed changes

vaithak force-pushed the vec-mode-call-expr branch from d96c3d6 to e04b101 Compare October 10, 2023 06:18

github-actions bot reviewed Oct 10, 2023

View reviewed changes

vaithak force-pushed the vec-mode-call-expr branch from e04b101 to 3c154c9 Compare October 10, 2023 09:23

vgvassilev requested a review from parth-07 October 25, 2023 10:20

vgvassilev added this to the v1.3 milestone Nov 4, 2023

vaithak force-pushed the vec-mode-call-expr branch 2 times, most recently from 4beb275 to 3d0d5e0 Compare November 4, 2023 20:52

vaithak closed this Nov 4, 2023

vaithak reopened this Nov 4, 2023

github-actions bot reviewed Nov 4, 2023

View reviewed changes

vaithak force-pushed the vec-mode-call-expr branch from 3d0d5e0 to 8ef4f4e Compare November 4, 2023 21:49

github-actions bot reviewed Nov 4, 2023

View reviewed changes

vaithak force-pushed the vec-mode-call-expr branch 2 times, most recently from 28b48c6 to 1241f64 Compare November 4, 2023 23:11

Add support for call expressions in vector forward mode AD

c0db619

vaithak force-pushed the vec-mode-call-expr branch from 1241f64 to c0db619 Compare December 2, 2023 14:16

vgvassilev approved these changes Dec 3, 2023

View reviewed changes

vgvassilev merged commit ec76b70 into vgvassilev:master Dec 3, 2023
75 checks passed

vaithak deleted the vec-mode-call-expr branch March 13, 2024 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for call expressions in vector forward mode AD #638

Add support for call expressions in vector forward mode AD #638

vaithak commented Oct 8, 2023 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

codecov bot commented Oct 10, 2023 •

edited

Loading

parth-07 commented Oct 27, 2023

vaithak commented Nov 4, 2023 •

edited

Loading

github-actions bot left a comment

github-actions bot Nov 4, 2023

github-actions bot Nov 4, 2023

github-actions bot left a comment

github-actions bot Nov 4, 2023

vgvassilev commented Dec 1, 2023

vgvassilev left a comment

Add support for call expressions in vector forward mode AD #638

Add support for call expressions in vector forward mode AD #638

Conversation

vaithak commented Oct 8, 2023 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 10, 2023 • edited Loading

Codecov Report

parth-07 commented Oct 27, 2023

vaithak commented Nov 4, 2023 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Nov 4, 2023

Choose a reason for hiding this comment

github-actions bot Nov 4, 2023

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Nov 4, 2023

Choose a reason for hiding this comment

vgvassilev commented Dec 1, 2023

vgvassilev left a comment

Choose a reason for hiding this comment

vaithak commented Oct 8, 2023 •

edited

Loading

codecov bot commented Oct 10, 2023 •

edited

Loading

vaithak commented Nov 4, 2023 •

edited

Loading