Change argument order on InplaceableThunk and fix deprecated tests

[oxinabox]

This closes #386
I will wait til we merge the ProjectTo PR in ChainRules.jl before making the follow up there.

While doing this I noticed out deprecated tests were not running so i fixed that and fixed some issues with that file

[codecov-commenter]

Codecov Report

Merging #396 (cd458a1) into master (4b33290) will increase coverage by 2.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #396      +/-   ##
==========================================
+ Coverage   89.96%   91.99%   +2.02%     
==========================================
  Files          15       15              
  Lines         638      637       -1     
==========================================
+ Hits          574      586      +12     
+ Misses         64       51      -13     

Impacted Files	Coverage Δ
src/deprecated.jl	100.00% <ø> (+100.00%)	⬆️
src/differentials/thunks.jl	94.89% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b33290...cd458a1. Read the comment docs.

[mzgubic]

good lord...

[findmyway]

Hi @oxinabox ,

I'm pretty sure this change triggered a very weird behavior in JuliaReinforcementLearning/ReinforcementLearning.jl#370 (comment)

Downgrading to v0.10.11 resolved that issue. I'd like to provide a MWE, but before spending time on it (it might need some effort), I'd like to know whether you can see any obvious potential errors in this PR or not.

(cc @pilgrimygy)

[oxinabox]

That's very weird.
This was a perfectly standard deprecation.

It that makes your project hang, then I blame the compiler.
If you can get s MWE then I think you will need to open an issue on JuliaLang/julia.
Probably using the bug reporting rr functionality.

[findmyway]

OK, here is the MWE: https://github.com/findmyway/TestCRC/blob/master/test/runtests.jl

As you can see, the performance difference is huge between v0.10.11 and v0.10.12:

ChainRulesCore@v0.10.12
BenchmarkTools.Trial: 62 samples with 1 evaluation.
 Range (min … max):  81.068 ms …  83.285 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     81.822 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   81.879 ms ± 473.558 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

ChainRulesCore@v0.10.11
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  22.937 μs …  4.882 ms  ┊ GC (min … max): 0.00% … 97.85%
 Time  (median):     23.713 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   25.517 μs ± 68.207 μs  ┊ GC (mean ± σ):  3.72% ±  1.39%

And the weirdest part is that the performance difference only shows up in the tests (by running ] test). When I test it in the REPL, there isn't any difference between these two versions. Since I'm quite new to ChainRules.jl, I really appreciate it if you could take a close look at it. (yeah I plan to learn it by watching @mzgubic's talk at JuliaCon 😆 )

[oxinabox]

Ah, yes, this is the deprecation warnings.
Deprecation warnings are disabled by default when running julia normally.
For a number of reasons, including that they are very slow.
But they are enabled during testings.
You can disable them by doing
Pkg.test(;julia_args=["--depwarn=no"])
I have no idea how to do that with GitHubActions or any other CI though.
Try asking on Discourse

A version of ChainRules.jl that doesn't have any deprecation warnings from this will come out sometime next week.
I am just waiting to merge a few other PRs first.

[findmyway]

Now I see. Thanks!