Skip to content

Conversation

@niklasschmitz
Copy link
Contributor

Porting nograd rules for one, ones, zero, zeros from Zygote https://github.com/FluxML/Zygote.jl/blob/master/src/lib/array.jl#L11

Follow-up to #252

@niklasschmitz
Copy link
Contributor Author

This was also mentioned in FluxML/Zygote.jl#780 by @oxinabox. Should these rather return ZeroTangent() than NoTangent() ?

@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2021

Codecov Report

Merging #465 (fa3ba9c) into master (93eb113) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #465   +/-   ##
=======================================
  Coverage   98.50%   98.50%           
=======================================
  Files          21       21           
  Lines        2201     2201           
=======================================
  Hits         2168     2168           
  Misses         33       33           
Impacted Files Coverage Δ
src/rulesets/Base/nondiff.jl 66.66% <ø> (ø)

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 93eb113...fa3ba9c. Read the comment docs.

@mzgubic
Copy link
Member

mzgubic commented Jul 2, 2021

I suppose one and zero need ZeroTangent() (see https://juliadiff.org/ChainRulesCore.jl/dev/FAQ.html#faq_abstract_zero)

but they work almost identically so I would favour the simplicity of using a macro over being pedantic in this case.

@mzgubic
Copy link
Member

mzgubic commented Jul 2, 2021

Thanks for the PR! Will merge when CI passes. Just FYI, it's possible to commit the code suggestions all at once, which prevents CI from running for each suggestion. I think there is a limit to how many jobs can be run per organisation at the same time, and ChainRules tests take a while so a queue forms sometimes.

@mzgubic mzgubic merged commit 5116c95 into JuliaDiff:master Jul 2, 2021
@niklasschmitz niklasschmitz deleted the nondiff-ones-zeros branch July 2, 2021 13:12
@niklasschmitz
Copy link
Contributor Author

Thanks @mzgubic !

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.

3 participants