Automatically provide tangents by oxinabox · Pull Request #116 · JuliaDiff/ChainRulesTestUtils.jl

oxinabox · 2021-02-03T19:48:13Z

Closes #67
Does option 2D witth x ⊢ ẋ
that is \vdash + tab
We can argue about symbols though.

input tangent
should make frule take an output tangent Make frule_test take a sample output tangent for testing purposes #66
should make frule and rrule's output tangent be auto-generated
Should probably also do renaming from Make naming consistent: rename test_scalar or frule_test+rrule_test #96 to test_frule and test_rrule. Which should make deprecations easier
tests

before:

x = rand(4, 5)
x̄ = rand(4, 5)

ȳ = rand(2, 10)

rrule_test(reshape, ȳ, (x, x̄), ((2, 10), nothing))
rrule_test(reshape, ȳ, (x, x̄), (2, nothing), (10, nothing))

After

No point declaring x in advance any-more.
Can just put the rand in-place

test_rrule(reshape, rand(4, 5), (2, 10) ⊢  nothing)
test_rrule(reshape, rand(4, 5), 2  ⊢  nothing, 10 ⊢  nothing)

oxinabox · 2021-02-04T19:36:02Z

I have not added tests for this explictly, but some places we still use this because we explictly want to pass special tangents in.
So we basically have real world based tests that this works.
So i think this is good.
but review should comment if they disagree

oxinabox · 2021-02-04T19:37:21Z

I would say that we should wait til ChainRules is updated to use this, before merging.
But it is going to be a big job to update all the tests. Probably multiple PRs.
So I think we should merge this, and then patch it if we find something was missed.

This is nonbreaking as the old way is deprecated.

src/generate_tangent.jl

src/ChainRulesTestUtils.jl

src/testers.jl

oxinabox · 2021-02-04T23:32:26Z

src/testers.jl

-function rrule_test(f, ȳ, xx̄s::Tuple{Any, Any}...; rtol::Real=1e-9, atol::Real=1e-9, fdm=_fdm, check_inferred::Bool=true, fkwargs::NamedTuple=NamedTuple(), kwargs...)
+function test_rrule(
+    f, inputs...;
+    output_tangent=Auto(),


Should this be output_cotangent?

oxinabox · 2021-02-04T23:34:13Z

src/testers.jl

    y = f(xs...; fkwargs...)
    check_equal(y_ad, y; isapprox_kwargs...)  # make sure primal is correct

+    ȳ = tangent(auto_primal_and_tangent(y ⊢ output_tangent))


It's a bit stilly to be attaching a tangent to the primal explicitly, then pealing it off, but it makes both given and Auto() work

Why don't we just use rand_tangent(y)? I don't understand this part of your comment:

but it makes both given and Auto() work

the alternative is:

Suggested change

ȳ = tangent(auto_primal_and_tangent(y ⊢ output_tangent))

ȳ = output_tangent === Auto() ? rand_tangent(primal) : output_tangent

Which maybe is cleaner?

idk yesterday I was hating on branches.
I started to write auto_tangent(primal, output_tangent) and realized that it overlapped with auto_primal_and_tangent

ȳ = output_tangent === Auto() ? rand_tangent(primal) : output_tangent is certainly much clearer, to my eyes

oxinabox · 2021-02-04T23:35:42Z

Ah, I forgot to push some changes.
Namely the ones that update all the tests
Will do so tomorrow

mzgubic · 2021-02-05T09:03:13Z

That's wonderful! Please ping me when it is done, happy to review

src/testers.jl

test/deprecated.jl

mzgubic

I love this change, makes the API so much easier to use. Should we also update the examples in the docs to reflect the new usage?

src/ChainRulesTestUtils.jl

src/deprecated.jl

src/generate_tangent.jl

src/testers.jl

mzgubic · 2021-02-05T11:54:21Z

src/testers.jl

    y = f(xs...; fkwargs...)
    check_equal(y_ad, y; isapprox_kwargs...)  # make sure primal is correct

+    ȳ = tangent(auto_primal_and_tangent(y ⊢ output_tangent))


Why don't we just use rand_tangent(y)? I don't understand this part of your comment:

but it makes both given and Auto() work

test/deprecated.jl

docs/src/index.md

Co-authored-by: Miha Zgubic <mzgubic@users.noreply.github.com>

src/testers.jl

CarloLucibello · 2021-02-06T10:01:03Z

I just got the deprecation message

┌ Warning: `rrule_test(f, ȳ, inputs::Tuple{Any, Any}...; kwargs...)` is deprecated, use `test_rrule(f, (x ⊢ dx for (x, dx) = inputs)...; output_tangent = ȳ, kwargs...)` instead.

and it looked a bit too exotic. Why not pairs => instead of vdash?

edit: maybe you need to differentiate with respect to a pair, I see now the problem

CarloLucibello · 2021-02-06T10:04:52Z

src/testers.jl

-If `check_inferred=true`, then the inferrability of the `rrule` is checked — if `f` is
-itself inferrable — along with the inferrability of the pullback it returns.
-All remaining keyword arguments are passed to `isapprox`.
+- `inputs` either the primal inputs `x`, or primals and their tangents: `x ⊢ ẋ`


ẋ should be x̄.
Maybe it should be explained also here how to produce a vdash

oxinabox · 2021-02-06T11:04:54Z

@CarloLucibello note the deprecations warning is misleading as to the correct action.
Correct is to just delete the partials and let the automatic process kick in.

nickrobinson251 · 2021-02-06T11:14:22Z

@CarloLucibello note the deprecations warning is misleading as to the correct action.
Correct is to just delete the partials and let the automatic process kick in.

Maybe manually define the deprecation, with a depwarn giving the better instructions, rather than rely on @deprecate

Otherwise people will do what the deprecation warning tells them to

oxinabox · 2021-02-06T12:57:37Z

Good idea, I will see if I find time

oxinabox marked this pull request as draft February 3, 2021 19:48

oxinabox mentioned this pull request Feb 4, 2021

Make work on Functors #117

Closed

oxinabox changed the title ~~WIP: automatically provide tangents~~ Automatically provide tangents Feb 4, 2021

oxinabox marked this pull request as ready for review February 4, 2021 19:35

willtebbutt reviewed Feb 4, 2021

View reviewed changes

src/generate_tangent.jl Show resolved Hide resolved

oxinabox commented Feb 4, 2021

View reviewed changes

oxinabox commented Feb 5, 2021

View reviewed changes

src/testers.jl Outdated Show resolved Hide resolved

oxinabox commented Feb 5, 2021

View reviewed changes

src/testers.jl Outdated Show resolved Hide resolved

oxinabox commented Feb 5, 2021

View reviewed changes

test/deprecated.jl Outdated Show resolved Hide resolved

oxinabox commented Feb 5, 2021

View reviewed changes

test/deprecated.jl Show resolved Hide resolved

oxinabox and others added 9 commits February 5, 2021 11:08

WIP: automatically provide tangents

0f7bac8

rename rule_test for test_rule, add autogentering output tangents

8c133a3

use vdash rather than perp as tangent seperator

cea1f44

don't use deprecated methods in test_scalar

8e3e69c

Remove deprecate items from tests

0b66cad

bump version

5a47fd8

Don't export Auto

6a62e8d

fix names of functions used in error messages

1c36f39

remove excess whitespace

3644e23

oxinabox force-pushed the ox/autotangent branch from d1b6746 to 3644e23 Compare February 5, 2021 11:12

mzgubic reviewed Feb 5, 2021

View reviewed changes

oxinabox added 3 commits February 5, 2021 12:59

improve docstrings

a770b9c

add to docs

66e76ac

use output tangent in frule

5388aeb

mzgubic approved these changes Feb 5, 2021

View reviewed changes

docs/src/index.md Outdated Show resolved Hide resolved

docs/src/index.md Outdated Show resolved Hide resolved

docs/src/index.md Outdated Show resolved Hide resolved

docs/src/index.md Outdated Show resolved Hide resolved

typos in docs

04564cc

Co-authored-by: Miha Zgubic <mzgubic@users.noreply.github.com>

oxinabox commented Feb 5, 2021

View reviewed changes

src/testers.jl Outdated Show resolved Hide resolved

oxinabox added 2 commits February 5, 2021 19:19

be less clever about output_tangent

b053cb9

Fix docs

bf8aa60

oxinabox merged commit 340e21c into master Feb 5, 2021

CarloLucibello reviewed Feb 6, 2021

View reviewed changes

oxinabox mentioned this pull request Feb 6, 2021

use new ChainRulesTestUtils JuliaMath/SpecialFunctions.jl#300

Merged

oxinabox deleted the ox/autotangent branch February 6, 2021 13:21

oxinabox mentioned this pull request Feb 11, 2021

Update tests to use new easier testing mechanism JuliaDiff/ChainRules.jl#360

Closed

18 tasks

mzgubic mentioned this pull request Apr 6, 2021

Make naming consistent: rename test_scalar or frule_test+rrule_test #96

Closed

nickrobinson251 mentioned this pull request Apr 6, 2021

rename check_equal -> test_approx #134

Closed

	ȳ = tangent(auto_primal_and_tangent(y ⊢ output_tangent))
	ȳ = output_tangent === Auto() ? rand_tangent(primal) : output_tangent

Conversation

oxinabox commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

before:

After

Uh oh!

oxinabox commented Feb 4, 2021

Uh oh!

oxinabox commented Feb 4, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oxinabox Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

oxinabox Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

mzgubic Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oxinabox Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

nickrobinson251 Feb 5, 2021

Choose a reason for hiding this comment

Uh oh!

oxinabox commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mzgubic commented Feb 5, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mzgubic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mzgubic Feb 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CarloLucibello commented Feb 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CarloLucibello Feb 6, 2021

Choose a reason for hiding this comment

Uh oh!

oxinabox commented Feb 6, 2021

Uh oh!

nickrobinson251 commented Feb 6, 2021

Uh oh!

oxinabox commented Feb 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

oxinabox commented Feb 3, 2021 •

edited

Loading

mzgubic Feb 5, 2021 •

edited

Loading

oxinabox commented Feb 4, 2021 •

edited

Loading

mzgubic Feb 5, 2021 •

edited

Loading

CarloLucibello commented Feb 6, 2021 •

edited

Loading