Add numerical differentiation support to clad #261

grimmmyshini · 2021-07-15T10:22:48Z

This functionality will allow clad to be able to use numerical differentiation as a backup (or even standalone) in the case that a differentiation request is made for a function that is not visible to clad. Full list of possible functionalities is as given below:

PR Checklist:

Add support for using numerical diff as a backup for unsupported function calls in forward-mode.
Add support for using numerical diff as a backup for unsupported function calls in reverse-mode.
Add support for usage of numerical diff as a standalone function (i.e. clad::numerical_diff).
Add support for numerical differentiation of multi-arg calls.
Extend support of numerical diff to differentiate unsupported operators.
Support printing of error information for the numerical methods.

Good to implement:

Add support for non-primitive types like user structs, tensors etc.

Known bugs:

Standalone code does not work because central_difference is not specialized correctly.

codecov · 2021-07-15T10:40:13Z

Codecov Report

Merging #261 (d20aedc) into master (b0fa87a) will increase coverage by 0.25%.
The diff coverage is 95.96%.

❗ Current head d20aedc differs from pull request most recent head 53db6d1. Consider uploading reports for the commit 53db6d1 to get more accurate results

@@            Coverage Diff             @@
##           master     #261      +/-   ##
==========================================
+ Coverage   88.20%   88.46%   +0.25%     
==========================================
  Files          28       28              
  Lines        3636     3735      +99     
==========================================
+ Hits         3207     3304      +97     
- Misses        429      431       +2

Impacted Files	Coverage Δ
include/clad/Differentiator/VisitorBase.h	`100.00% <ø> (ø)`
lib/Differentiator/ForwardModeVisitor.cpp	`90.87% <87.50%> (-0.36%)`	⬇️
lib/Differentiator/ReverseModeVisitor.cpp	`90.97% <96.15%> (+0.31%)`	⬆️
include/clad/Differentiator/DerivativeBuilder.h	`100.00% <100.00%> (ø)`
lib/Differentiator/DerivativeBuilder.cpp	`100.00% <100.00%> (ø)`
lib/Differentiator/VisitorBase.cpp	`96.28% <100.00%> (+0.60%)`	⬆️
tools/ClangPlugin.cpp	`88.97% <100.00%> (+0.16%)`	⬆️
tools/ClangPlugin.h	`73.21% <100.00%> (+1.51%)`	⬆️

Impacted Files	Coverage Δ
include/clad/Differentiator/VisitorBase.h	`100.00% <ø> (ø)`
lib/Differentiator/ForwardModeVisitor.cpp	`90.87% <87.50%> (-0.36%)`	⬇️
lib/Differentiator/ReverseModeVisitor.cpp	`90.97% <96.15%> (+0.31%)`	⬆️
include/clad/Differentiator/DerivativeBuilder.h	`100.00% <100.00%> (ø)`
lib/Differentiator/DerivativeBuilder.cpp	`100.00% <100.00%> (ø)`
lib/Differentiator/VisitorBase.cpp	`96.28% <100.00%> (+0.60%)`	⬆️
tools/ClangPlugin.cpp	`88.97% <100.00%> (+0.16%)`	⬆️
tools/ClangPlugin.h	`73.21% <100.00%> (+1.51%)`	⬆️

vgvassilev

Could you add tests for the diagnostics branches? Codecov tells where they are.

demos/CustomTypeNumDiff.cpp

include/clad/Differentiator/DerivativeBuilder.h

include/clad/Differentiator/NumericalDiff.h

include/clad/Differentiator/VisitorBase.h

lib/Differentiator/VisitorBase.cpp

test/lit.cfg

grimmmyshini · 2021-08-18T12:27:54Z

Could you add tests for the diagnostics branches? Codecov tells where they are.

That is blocked right now. When clad fails to derive a function, it crashes. Let me put up an issue for this soon.

vgvassilev

The more I think the more I convince myself that PR, at some stage, should turn into a callback class based implementation. It is an opt-in piece and also somewhat an AD bail out plan. Let's resolve the comments we got so far and discuss how to move forward given the limited amount of time we have left.

demos/CustomTypeNumDiff.cpp

include/clad/Differentiator/NumericalDiff.h

lib/Differentiator/ForwardModeVisitor.cpp

lib/Differentiator/ReverseModeVisitor.cpp

vgvassilev · 2021-08-19T19:41:31Z

lib/Differentiator/ReverseModeVisitor.cpp

+          // Try numerically deriving it.
+          if (NArgs == 1) {
+            // Build a clone call expression so that we can correctly
+            // scope the function to be differentiated.
+            Expr* call = m_Sema
+                       .ActOnCallExpr(getCurrentScope(),
+                                      Clone(CE->getCallee()),
+                                      noLoc,
+                                      llvm::MutableArrayRef<Expr*>(CallArgs),
+                                      noLoc)
+                       .get();
+            Expr* fnCallee = cast<CallExpr>(call)->getCallee();
+            OverloadedDerivedFn = GetSingleArgCentralDiffCall(fnCallee,
+                                                              DerivedCallArgs
+                                                                  [0],
+                                                              0, 1,
+                                                              DerivedCallArgs);
+            asGrad = !OverloadedDerivedFn;
+          } else {
+            // Build a clone call expression so that we can correctly
+            // scope the function to be differentiated.
+            Expr* call = m_Sema
+                             .ActOnCallExpr(getCurrentScope(),
+                                            Clone(CE->getCallee()), noLoc,
+                                            llvm::MutableArrayRef<Expr*>(
+                                                CallArgs),
+                                            noLoc)
+                             .get();
+            Expr* fnCallee = cast<CallExpr>(call)->getCallee();
+            OverloadedDerivedFn = GetMultiArgCentralDiffCall(
+                fnCallee, CEType.getCanonicalType(), CE->getNumArgs(),
+                NumericalDiffMultiArg, DerivedCallArgs, DerivedCallOutputArgs);
+          }
+          if (!OverloadedDerivedFn) {
+            // Function was not derived => issue a warning.
+            diag(DiagnosticsEngine::Warning,
+                 CE->getBeginLoc(),
+                 "function '%0' was not differentiated because clad failed to "
+                 "differentiate it and no suitable overload was found in "
+                 "namespace 'custom_derivatives', and function may not be "
+                  "eligible for numerical differentiation.",
+                 {FD->getNameAsString()});
+            return StmtDiff(Clone(CE));
+          } else {
+            diag(DiagnosticsEngine::Remark, noLoc,
+                 "Falling back to numerical differentiation for '%0' since no "
+                 "suitable overload was found and clad could not derive it. "
+                 "To disable this feature, compile your programs with " 
+                 "-DCLAD_NO_NUM_DIFF.",
+                 {FD->getNameAsString()});
+          }
+        } else {
+          OverloadedDerivedFn = m_Sema
+                                    .ActOnCallExpr(getCurrentScope(),
+                                                   BuildDeclRef(derivedFD),
+                                                   noLoc,
+                                                   llvm::MutableArrayRef<Expr*>(
+                                                       DerivedCallArgs),
+                                                   noLoc)
+                                    .get();
+        }    


Can you move this into a separate function?

The whole thing? I don't know if that would be feasible...We will have to send a lot of information as parameters to that function.

How about putting 1406 - 1423 in a function? Putting all of it will require a function with 8-9 arguments.

@vgvassilev How do you think it looks now? If it is not good, I can put it all in a function.

vgvassilev · 2021-08-19T19:44:44Z

lib/Differentiator/ForwardModeVisitor.cpp

+                                      noLoc,
+                                      llvm::MutableArrayRef<Expr*>(CallArgs),
+                                      noLoc)
+                       .get();


Consider moving into a separate function.

include/clad/Differentiator/NumericalDiff.h

vgvassilev · 2021-08-20T19:48:33Z

include/clad/Differentiator/NumericalDiff.h

+  }
+
+  /// A funtion to calculate the derivative of a function using the central
+  /// difference formula. Note: we do not propogate errors resulting in the


Can you elaborate here and maybe in the comments.

include/clad/Differentiator/NumericalDiff.h

lib/Differentiator/ForwardModeVisitor.cpp

vgvassilev · 2021-08-22T05:57:20Z

tools/ClangPlugin.cpp

@@ -179,6 +179,12 @@ namespace clad {
              estimationPlugin->InstantiateCustomModel(*m_DerivativeBuilder));
        }
      }
+
+      // If enabled, set the proper fields in derivative builder.
+      if (m_DO.PrintNumDiffErrorInfo) {


Can you add a test here?

test/Jacobian/Pointers.C

Clad can now build calls to a "forward" (single-arg) and "reverse" (multi-arg) numerical diff function. Numerical diff can be used in the following contexts: - With clad forward and reverse mode for all in-built scalar and non scalar types. - With clad and standalone for "forward" and "reverse" derivatives. - Standalone for in-built scalar and non-scalar types. - Standalone with support for user defined types as input (both value and pointer forms). - Standalone to differentiate overloaded operators. - Standalone to differentiate functors. Numerical diff error estimates can be printed with the help of the '-fprint-num-diff-errors' flag, and numerical diff may be disabled using the '-DCLAD_NO_NUM_DIFF' flag during compilation.

grimmmyshini force-pushed the central-diff branch from 7b4e1bd to ac70839 Compare August 13, 2021 18:28

grimmmyshini marked this pull request as ready for review August 13, 2021 18:35

grimmmyshini force-pushed the central-diff branch 2 times, most recently from 7a9ebec to e9bc67d Compare August 17, 2021 16:09

vgvassilev reviewed Aug 17, 2021

View reviewed changes

vgvassilev mentioned this pull request Aug 18, 2021

Add support for automatic testing of reverse mode using forward mode #290

Draft

2 tasks

vgvassilev reviewed Aug 19, 2021

View reviewed changes

include/clad/Differentiator/NumericalDiff.h Outdated Show resolved Hide resolved

vgvassilev reviewed Aug 20, 2021

View reviewed changes

grimmmyshini mentioned this pull request Aug 21, 2021

[Feature Request] Floating Point Error Estimation Framework to do checklist #295

Closed

5 tasks

grimmmyshini force-pushed the central-diff branch from dfb7b2d to 66d6b03 Compare August 21, 2021 20:17

vgvassilev reviewed Aug 22, 2021

View reviewed changes

grimmmyshini force-pushed the central-diff branch from d20aedc to b709ff1 Compare August 22, 2021 12:57

grimmmyshini force-pushed the central-diff branch from b709ff1 to 53db6d1 Compare August 22, 2021 13:00

vgvassilev merged commit 4f1c10e into vgvassilev:master Aug 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add numerical differentiation support to clad #261

Add numerical differentiation support to clad #261

grimmmyshini commented Jul 15, 2021 •

edited

codecov bot commented Jul 15, 2021 •

edited

vgvassilev left a comment

grimmmyshini commented Aug 18, 2021

vgvassilev left a comment

vgvassilev Aug 19, 2021

grimmmyshini Aug 19, 2021

grimmmyshini Aug 21, 2021

grimmmyshini Aug 21, 2021

vgvassilev Aug 19, 2021

vgvassilev Aug 20, 2021

vgvassilev Aug 22, 2021

grimmmyshini Aug 22, 2021

Add numerical differentiation support to clad #261

Add numerical differentiation support to clad #261

Conversation

grimmmyshini commented Jul 15, 2021 • edited

codecov bot commented Jul 15, 2021 • edited

Codecov Report

vgvassilev left a comment

Choose a reason for hiding this comment

grimmmyshini commented Aug 18, 2021

vgvassilev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grimmmyshini commented Jul 15, 2021 •

edited

codecov bot commented Jul 15, 2021 •

edited