Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests supporting higher order custom derivatives #786

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

aaronj0
Copy link
Collaborator

@aaronj0 aaronj0 commented Feb 24, 2024

Fixes #313

@aaronj0 aaronj0 added this to the v1.4 milestone Feb 24, 2024
Copy link

codecov bot commented Feb 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.71%. Comparing base (4967d9a) to head (818c8dd).
Report is 2 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #786   +/-   ##
=======================================
  Coverage   94.71%   94.71%           
=======================================
  Files          49       49           
  Lines        7342     7342           
=======================================
  Hits         6954     6954           
  Misses        388      388           

@aaronj0 aaronj0 removed this from the v1.4 milestone Feb 24, 2024
@parth-07
Copy link
Collaborator

This looks good. I am surprised we didn't have any test for higher-order derivatives of built-in custom derivatives.

Can you please see if is is possible to use functions available in test/TestUtils.h for testing?

// CHECK-NEXT: clad::ValueAndPushforward<ValueAndPushforward<float, float>, ValueAndPushforward<float, float> > _t0 = sin_pushforward_pushforward(x * y, _d_x0 * y + x * _d_y0, _d_x * y + x * _d_y, _d__d_x * y + _d_x0 * _d_y + _d_x * _d_y0 + x * _d__d_y);
// CHECK-NEXT: ValueAndPushforward<float, float> _d__t0 = _t0.pushforward;
// CHECK-NEXT: ValueAndPushforward<float, float> _t00 = _t0.value;
// CHECK-NEXT: clad::ValueAndPushforward<ValueAndPushforward<decltype(::std::pow(float(), int())), decltype(::std::pow(float(), int()))>, ValueAndPushforward<decltype(::std::pow(float(), int())), decltype(::std::pow(float(), int()))> > _t1 = pow_pushforward_pushforward(_t00.value, a, _t00.pushforward, _d_a0, _d__t0.value, _d_a, _d__t0.pushforward, _d__d_a);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably need regex-based checks here because the calls differ for libstdc++ (unix) and libc++ (darwin).

Copy link
Collaborator Author

@aaronj0 aaronj0 Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parth-07 the utilities in TestUtils.h don't have a template parameter field for the order of differentiation.

i did change the macro to allow this:

#define INIT_DIFFERENTIATE(fn, order, ...)                                     \
  auto fn##_diff = clad::differentiate<order>(fn, __VA_ARGS__);

But now the problem is that it can't be used for the same function twice:

INIT_DIFFERENTIATE(test_trig, 2, "x");
TEST_DIFFERENTIATE(test_trig, 0.5, 1.0, 1, 2); // CHECK-EXEC: -2.364220

INIT_DIFFERENTIATE(test_trig, 2, "y");
TEST_DIFFERENTIATE(test_trig, 1.0, 0.5, 2, 1); // CHECK-EXEC: -0.060237

fails because the macro redefines test_trig_diff

I can fix this by appending the vars and order to the generated function like so (just a psuedo code for the name generation)

auto fn##_##str(order)##_diff = clad::differentiate<order>(fn, __VA_ARGS__);

although I'm not sure if thats the right way to generate the name

This can be done in another PR since if implemented it will require refactoring of other tests, so can we go ahead with adding the test as is for now?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move forward as it is since our release due date is tomorrow.

@vgvassilev vgvassilev merged commit 2f87089 into vgvassilev:master Feb 28, 2024
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support higher order custom derivatives
3 participants