Add support for differentiation of immediate functions #1109

MihailMihov · 2024-10-03T12:38:19Z

No description provided.

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-03T12:49:54Z

include/clad/Differentiator/Differentiator.h

  class CladFunction {
  public:
    using CladFunctionType = F;
    using FunctorType = FunctorT;

  private:
    CladFunctionType m_Function;
-    char* m_Code;
+    const char* m_Code;
    FunctorType *m_Functor = nullptr;
    bool m_CUDAkernel = false;

  public:
    CUDA_HOST_DEVICE CladFunction(CladFunctionType f, const char* code,
                                  FunctorType* functor = nullptr,
                                  bool CUDAkernel = false)


warning: expected ';' at end of declaration list [clang-diagnostic-error]

Suggested change

bool CUDAkernel = false)

bool CUDAkernel = false);

github-actions · 2024-10-03T12:49:55Z

include/clad/Differentiator/Differentiator.h

    /// Constructor overload for initializing `m_Functor` when functor
    /// is passed by reference.
    CUDA_HOST_DEVICE
    CladFunction(CladFunctionType f, const char* code, FunctorType& functor)
        : CladFunction(f, code, &functor) {};

+    constexpr CUDA_HOST_DEVICE CladFunction(CladFunctionType f,


warning: constructor does not initialize these fields: m_Code [cppcoreguidelines-pro-type-member-init]

include/clad/Differentiator/Differentiator.h:203:

- const char* m_Code; + const char* m_Code{};

github-actions · 2024-10-03T12:49:55Z

include/clad/Differentiator/Differentiator.h

      return static_cast<return_type_t<F>>(0);
    }

    /// Return the string representation for the generated derivative.
-    const char* getCode() const {
+    constexpr const char* getCode() const {


warning: function 'getCode' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change

constexpr const char* getCode() const {

[[nodiscard]] constexpr const char* getCode() const {

github-actions · 2024-10-03T12:49:55Z

include/clad/Differentiator/Differentiator.h

+  template <unsigned... BitMaskedOpts, typename ArgSpec = const char*,
+            typename F, typename... Args>
+  return_type_t<F> constexpr differentiate_and_execute(F fn, ArgSpec args = "",
+                                                       Args&&... args_) {


warning: invalid case style for parameter 'args_' [readability-identifier-naming]

Suggested change

Args&&... args_) {

Args&&... args) {

include/clad/Differentiator/Differentiator.h:474:

- std::forward<Args>(args_)...); + std::forward<Args>(args)...);

github-actions · 2024-10-03T12:49:55Z

lib/Differentiator/DiffPlanner.cpp

-      call->setArg(codeArgIdx, newArg);
+    if (derivedFnArgIdx != -1) {
+      // Add the "&" operator
+      auto newUnOp =


warning: 'auto newUnOp' can be declared as 'auto *newUnOp' [llvm-qualified-auto]

Suggested change

auto newUnOp =

auto *newUnOp =

That seems a good suggestion.

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-16T18:35:23Z

include/clad/Differentiator/Differentiator.h

+  template <unsigned... BitMaskedOpts, typename ArgSpec = const char*,
+            typename F, typename... Args>
+  return_type_t<F> constexpr differentiate_and_execute(F fn, ArgSpec args = "",
+                                                       Args&&... args_) {


warning: invalid case style for parameter 'args_' [readability-identifier-naming]

Suggested change

Args&&... args_) {

Args&&... args) {

include/clad/Differentiator/Differentiator.h:471:

- std::forward<Args>(args_)...); + std::forward<Args>(args)...);

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-19T14:03:10Z

include/clad/Differentiator/ArrayRef.h

-  CUDA_HOST_DEVICE std::size_t size() const { return m_size; }
-  CUDA_HOST_DEVICE PUREFUNC T* ptr() const { return m_arr; }
-  CUDA_HOST_DEVICE PUREFUNC T*& ptr_ref() { return m_arr; }
+  constexpr CUDA_HOST_DEVICE std::size_t size() const { return m_size; }


warning: function 'size' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change

constexpr CUDA_HOST_DEVICE std::size_t size() const { return m_size; }

[[nodiscard]] constexpr CUDA_HOST_DEVICE std::size_t size() const { return m_size; }

github-actions · 2024-10-19T14:03:11Z

include/clad/Differentiator/Differentiator.h

+                                  bool CUDAkernel = false)
+        : m_Function(f), m_Functor(functor), m_CUDAkernel(CUDAkernel) {
+#ifndef __CLAD_SO_LOADED
+      static_assert(false, "clad doesn't appear to be loaded; make sure that "


warning: static assertion failed: clad doesn't appear to be loaded; make sure that you pass clad.so to clang. [clang-diagnostic-error]

static_assert(false, "clad doesn't appear to be loaded; make sure that " ^

github-actions · 2024-10-19T14:03:11Z

include/clad/Differentiator/Differentiator.h

    /// Constructor overload for initializing `m_Functor` when functor
    /// is passed by reference.
-    CUDA_HOST_DEVICE
-    CladFunction(CladFunctionType f, const char* code, FunctorType& functor)
+    CUDA_HOST_DEVICE CladFunction(CladFunctionType f, const char* code,


warning: constructor does not initialize these fields: m_Code [cppcoreguidelines-pro-type-member-init]

include/clad/Differentiator/Differentiator.h:203:

- const char* m_Code; + const char* m_Code{};

github-actions · 2024-10-19T14:03:11Z

include/clad/Differentiator/Differentiator.h

        : CladFunction(f, code, &functor) {};

+    constexpr CUDA_HOST_DEVICE CladFunction(CladFunctionType f,


warning: constructor does not initialize these fields: m_Code [cppcoreguidelines-pro-type-member-init]

constexpr CUDA_HOST_DEVICE CladFunction(CladFunctionType f, ^

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-21T20:34:57Z

include/clad/Differentiator/DiffPlanner.h

@@ -65,6 +65,9 @@ struct DiffRequest {
  /// A flag to enable TBR analysis during reverse-mode differentiation.
  bool EnableTBRAnalysis = false;
  bool EnableVariedAnalysis = false;
+  /// A flag specifying whether this differentiation is to be used
+  /// in immediate contexts.
+  bool ImmediateMode = false;


warning: member variable 'ImmediateMode' has public visibility [cppcoreguidelines-non-private-member-variables-in-classes]

bool ImmediateMode = false; ^

codecov · 2024-10-25T19:51:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.44%. Comparing base (21e4b4f) to head (27f1892).
Report is 11 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1109      +/-   ##
==========================================
+ Coverage   94.43%   94.44%   +0.01%     
==========================================
  Files          50       50              
  Lines        8751     8770      +19     
==========================================
+ Hits         8264     8283      +19     
  Misses        487      487

Files with missing lines	Coverage Δ
include/clad/Differentiator/CladConfig.h	`100.00% <ø> (ø)`
include/clad/Differentiator/DiffPlanner.h	`68.18% <ø> (ø)`
include/clad/Differentiator/DynamicGraph.h	`84.21% <100.00%> (+0.28%)`	⬆️
lib/Differentiator/DiffPlanner.cpp	`98.64% <100.00%> (+0.04%)`	⬆️
tools/ClangPlugin.cpp	`96.13% <100.00%> (+0.05%)`	⬆️

Files with missing lines	Coverage Δ
include/clad/Differentiator/CladConfig.h	`100.00% <ø> (ø)`
include/clad/Differentiator/DiffPlanner.h	`68.18% <ø> (ø)`
include/clad/Differentiator/DynamicGraph.h	`84.21% <100.00%> (+0.28%)`	⬆️
lib/Differentiator/DiffPlanner.cpp	`98.64% <100.00%> (+0.04%)`	⬆️
tools/ClangPlugin.cpp	`96.13% <100.00%> (+0.05%)`	⬆️

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-25T19:52:13Z

include/clad/Differentiator/Differentiator.h

    /// Constructor overload for initializing `m_Functor` when functor
    /// is passed by reference.
-    CUDA_HOST_DEVICE
-    CladFunction(CladFunctionType f, const char* code, FunctorType& functor)
+    CUDA_HOST_DEVICE CladFunction(CladFunctionType f, const char* code,


warning: constructor does not initialize these fields: m_Code [cppcoreguidelines-pro-type-member-init]

include/clad/Differentiator/Differentiator.h:202:

- const char* m_Code; + const char* m_Code{};

include/clad/Differentiator/DynamicGraph.h

vgvassilev · 2024-10-27T12:06:25Z

tools/ClangPlugin.cpp

+      for (DiffRequest& request : m_DiffRequestGraph.getNodes()) {
+        if (!request.ImmediateMode ||
+            (!request.Function->isConstexpr() &&
+             !request.Function->isImmediateFunction()))
+          continue;
+
+        m_DiffRequestGraph.setCurrentProcessingNode(request);
+        ProcessDiffRequest(request);
+        m_DiffRequestGraph.markCurrentNodeProcessed();


Suggested change

for (DiffRequest& request : m_DiffRequestGraph.getNodes()) {

if (!request.ImmediateMode ||

(!request.Function->isConstexpr() &&

!request.Function->isImmediateFunction()))

continue;

m_DiffRequestGraph.setCurrentProcessingNode(request);

ProcessDiffRequest(request);

m_DiffRequestGraph.markCurrentNodeProcessed();

for (DiffRequest& request : m_DiffRequestGraph.getNodes()) {

if (request.ImmediateMode &&

(request.Function->isConstexpr() ||

request.Function->isImmediateFunction())) {

m_DiffRequestGraph.setCurrentProcessingNode(request);

ProcessDiffRequest(request);

m_DiffRequestGraph.markCurrentNodeProcessed();

}

I am not sure why we need to iterate over all of the requests, though...

In this method HandleTopLevelDeclForClad we are given the new DeclGroupRef and then the DiffCollector adds the new nodes to the differentiation graph, but we don't really have a way to see what is new. I don't think it would be correct to move the processing part inside the collector as that's not it's purpose. Maybe there should be some way for the collector to give references to just the nodes that it added on it's last traversal? Should I do that or do you have something else in mind?

I am wondering if we shouldn't check if there is a FunctionDecl in DGR and ask it if it were an immediate function?

DGR is the top level decl, so to find the just the immediate FunctionDecls we'd need to traverse all of it again, which doesn't sound like a great idea, when that's what DiffCollector already does, or am I misunderstanding?

Ah, yes you are right. Please then just invert the condition as suggested and I think we might be good to go.

Okay, I'll do that, but also I think that I'll have some documentation ready by tomorrow, so maybe I can include that in this PR.

Agreed. We need a demo but that can be in a separate PR.

include/clad/Differentiator/Differentiator.h

vgvassilev · 2024-10-29T17:08:14Z

docs/userDocs/source/user/UsingImmediateMode.rst

+The derivatives that Clad generates are valid C++ code, which
+could in theory be executed at compile-time (or in an immediate
+context as the C++ standard calls it). When a function is differentiated
+all attributes, such as `constexpr` and `consteval` are kept, but


Suggested change

all attributes, such as `constexpr` and `consteval` are kept, but

all specifiers, such as `constexpr` and `consteval` are kept, but

vgvassilev · 2024-10-29T17:09:16Z

docs/userDocs/source/user/UsingImmediateMode.rst

+Usage of Clad's immediate mode
+================================================
+
+The following code snippet shows how one can request Clad to use the immediate mode for


We tend to break at 80 columns.

vgvassilev · 2024-10-29T17:10:17Z

docs/userDocs/source/user/UsingImmediateMode.rst

+loops, etc. are not yet usable in an immediate context.
+
+Both `constexpr` and `consteval` are supported as Clad doesn't actually rely on these
+specific keywords for it's support, but instead uses clang's API to determine if the functions


Suggested change

specific keywords for it's support, but instead uses clang's API to determine if the functions

specific keywords for its support, but instead uses clang's API to determine if the functions

vgvassilev · 2024-10-29T17:11:57Z

tools/ClangPlugin.cpp

+      for (DiffRequest& request : m_DiffRequestGraph.getNodes()) {
+        if (request.ImmediateMode &&
+            (request.Function->isConstexpr() ||
+             request.Function->isImmediateFunction())) {


Any chance to write a test that hits here?

It seems like isImmediateFunction isn't being hit, because isConstexpr is checking the ConstexprSpecKind, which contains information for constexpr, consteval and constinit and even if I remove the isImmediateFunction check the tests still pass, but I believe that is only the case, because with the current implementation we are guaranteed that clang would've set the ConstexprSpecKind before we process the FunctionDecl. So would it be better to keep that assumption and remove the second check or keep it, even though it's not tested currently?

Yeah. Removing the check makes sense.

vgvassilev

Lgtm!

MihailMihov force-pushed the constexpr-support branch from bf49849 to d5c0602 Compare October 3, 2024 12:40

github-actions bot reviewed Oct 3, 2024

View reviewed changes

vgvassilev force-pushed the constexpr-support branch from d5c0602 to 7476842 Compare October 15, 2024 19:47

MihailMihov force-pushed the constexpr-support branch from 7476842 to 866081e Compare October 16, 2024 18:25

github-actions bot reviewed Oct 16, 2024

View reviewed changes

MihailMihov force-pushed the constexpr-support branch 2 times, most recently from 3b4cbd3 to 26d7071 Compare October 19, 2024 13:52

github-actions bot reviewed Oct 19, 2024

View reviewed changes

MihailMihov force-pushed the constexpr-support branch 3 times, most recently from d5954b9 to c213322 Compare October 21, 2024 20:23

github-actions bot reviewed Oct 21, 2024

View reviewed changes

MihailMihov force-pushed the constexpr-support branch 3 times, most recently from 482d0dc to fa19625 Compare October 23, 2024 14:25

MihailMihov marked this pull request as ready for review October 23, 2024 15:12

MihailMihov force-pushed the constexpr-support branch from fa19625 to ef992db Compare October 23, 2024 21:13

github-actions bot reviewed Oct 25, 2024

View reviewed changes

vgvassilev reviewed Oct 27, 2024

View reviewed changes

MihailMihov force-pushed the constexpr-support branch from 1c243ec to 28343d5 Compare October 27, 2024 12:31

vgvassilev force-pushed the constexpr-support branch from 28343d5 to 9fdea68 Compare October 27, 2024 18:48

MihailMihov force-pushed the constexpr-support branch 2 times, most recently from 515d0ba to 6d1e43e Compare October 27, 2024 21:31

vgvassilev reviewed Oct 29, 2024

View reviewed changes

MihailMihov force-pushed the constexpr-support branch from 38f708f to 1ec232a Compare October 30, 2024 09:53

MihailMihov and others added 4 commits October 30, 2024 11:30

CladFunction constexpr

0638cc9

Process DiffRequests to immediate functions earlier

b13cc77

Add tests for constexpr and consteval

34eccfc

Do not assume index of derivedFn and code parameters

c213194

Mark clad::array_ref methods as constexpr

294aea7

MihailMihov force-pushed the constexpr-support branch from 1ec232a to c8395b0 Compare October 30, 2024 10:30

MihailMihov and others added 6 commits October 30, 2024 14:22

Keep track of whether a request is immediate in DiffRequest

24dbec6

Fix ForwardMode/NotEnoughArgError.C

b9d5bbf

Fix cuda device host constexpr execute functions declarations

7e7aa96

Store argPtrs of cuda kernels in a std array instead of a vector

d7dcb34

Rename __CLAD_SO_LOADED to __CLAD__

21f62ce

Add page to documentation explaining clad::immediate_mode

27f1892

MihailMihov force-pushed the constexpr-support branch from c8395b0 to 27f1892 Compare October 30, 2024 13:22

vgvassilev approved these changes Oct 30, 2024

View reviewed changes

vgvassilev merged commit cddc21d into vgvassilev:master Oct 30, 2024
90 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for differentiation of immediate functions #1109

Add support for differentiation of immediate functions #1109

MihailMihov commented Oct 3, 2024

github-actions bot left a comment

github-actions bot Oct 3, 2024

github-actions bot Oct 3, 2024

github-actions bot Oct 3, 2024

github-actions bot Oct 3, 2024

github-actions bot Oct 3, 2024

vgvassilev Oct 27, 2024

github-actions bot left a comment

github-actions bot Oct 16, 2024

github-actions bot left a comment

github-actions bot Oct 19, 2024

github-actions bot Oct 19, 2024

github-actions bot Oct 19, 2024

github-actions bot Oct 19, 2024

github-actions bot left a comment

github-actions bot Oct 21, 2024

codecov bot commented Oct 25, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot Oct 25, 2024

vgvassilev Oct 27, 2024

MihailMihov Oct 27, 2024

vgvassilev Oct 27, 2024

MihailMihov Oct 27, 2024

vgvassilev Oct 27, 2024

MihailMihov Oct 27, 2024

vgvassilev Oct 28, 2024

vgvassilev Oct 29, 2024

vgvassilev Oct 29, 2024

vgvassilev Oct 29, 2024

vgvassilev Oct 29, 2024

MihailMihov Oct 30, 2024

vgvassilev Oct 30, 2024

vgvassilev left a comment

	constexpr const char* getCode() const {
	[[nodiscard]] constexpr const char* getCode() const {

	constexpr CUDA_HOST_DEVICE std::size_t size() const { return m_size; }
	[[nodiscard]] constexpr CUDA_HOST_DEVICE std::size_t size() const { return m_size; }

		: CladFunction(f, code, &functor) {};

		constexpr CUDA_HOST_DEVICE CladFunction(CladFunctionType f,

	all attributes, such as `constexpr` and `consteval` are kept, but
	all specifiers, such as `constexpr` and `consteval` are kept, but

	specific keywords for it's support, but instead uses clang's API to determine if the functions
	specific keywords for its support, but instead uses clang's API to determine if the functions

Add support for differentiation of immediate functions #1109

Add support for differentiation of immediate functions #1109

Conversation

MihailMihov commented Oct 3, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 3, 2024

Choose a reason for hiding this comment

github-actions bot Oct 3, 2024

Choose a reason for hiding this comment

github-actions bot Oct 3, 2024

Choose a reason for hiding this comment

github-actions bot Oct 3, 2024

Choose a reason for hiding this comment

github-actions bot Oct 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 16, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 19, 2024

Choose a reason for hiding this comment

github-actions bot Oct 19, 2024

Choose a reason for hiding this comment

github-actions bot Oct 19, 2024

Choose a reason for hiding this comment

github-actions bot Oct 19, 2024

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 21, 2024

Choose a reason for hiding this comment

codecov bot commented Oct 25, 2024 • edited Loading

Codecov Report

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 25, 2024

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

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

vgvassilev left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 25, 2024 •

edited

Loading